Skip to content

Commit

Permalink
Use the correct value when the discriminator is part of the key.
Browse files Browse the repository at this point in the history
Optimize the discriminator value in the runtime model

Fixes #18126
  • Loading branch information
AndriySvyryd authored Sep 22, 2021
1 parent 3cc19f3 commit 3b67c3d
Show file tree
Hide file tree
Showing 10 changed files with 54 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
5 changes: 1 addition & 4 deletions src/EFCore/ChangeTracking/Internal/ValueGenerationManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/EFCore/Metadata/Conventions/RuntimeModelConvention.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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 =>
Expand Down Expand Up @@ -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);
Expand Down
10 changes: 9 additions & 1 deletion src/EFCore/Metadata/RuntimeEntityType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ private readonly SortedDictionary<string, RuntimeServiceProperty> _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;
Expand Down Expand Up @@ -89,7 +90,8 @@ public RuntimeEntityType(
string? discriminatorProperty,
ChangeTrackingStrategy changeTrackingStrategy,
PropertyInfo? indexerPropertyInfo,
bool propertyBag)
bool propertyBag,
object? discriminatorValue)
{
Name = name;
_clrType = type;
Expand All @@ -104,6 +106,7 @@ public RuntimeEntityType(
_indexerPropertyInfo = indexerPropertyInfo;
_isPropertyBag = propertyBag;
SetAnnotation(CoreAnnotationNames.DiscriminatorProperty, discriminatorProperty);
_discriminatorValue = discriminatorValue;

_properties = new SortedDictionary<string, RuntimeProperty>(new PropertyNameComparer(this));
}
Expand Down Expand Up @@ -860,6 +863,11 @@ ChangeTrackingStrategy IReadOnlyEntityType.GetChangeTrackingStrategy()
return (string?)this[CoreAnnotationNames.DiscriminatorProperty];
}

/// <inheritdoc/>
[DebuggerStepThrough]
object? IReadOnlyEntityType.GetDiscriminatorValue()
=> _discriminatorValue;

/// <inheritdoc/>
bool IReadOnlyTypeBase.HasSharedClrType
{
Expand Down
7 changes: 5 additions & 2 deletions src/EFCore/Metadata/RuntimeModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
/// </param>
/// <param name="discriminatorValue">the discriminator value for this entity type.</param>
/// <returns> The new entity type. </returns>
public virtual RuntimeEntityType AddEntityType(
string name,
Expand All @@ -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,
Expand All @@ -89,7 +91,8 @@ public virtual RuntimeEntityType AddEntityType(
discriminatorProperty,
changeTrackingStrategy,
indexerPropertyInfo,
propertyBag);
propertyBag,
discriminatorValue);

if (sharedClrType)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@ public class DiscriminatorValueGeneratorFactory : ValueGeneratorFactory
{
/// <inheritdoc />
public override ValueGenerator Create(IProperty property, IEntityType entityType)
=> new DiscriminatorValueGenerator(entityType.GetDiscriminatorValue()!);
=> new DiscriminatorValueGenerator();
}
}
17 changes: 3 additions & 14 deletions src/EFCore/ValueGeneration/Internal/DiscriminatorValueGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -13,27 +15,14 @@ namespace Microsoft.EntityFrameworkCore.ValueGeneration.Internal
/// </summary>
public class DiscriminatorValueGenerator : ValueGenerator
{
private readonly object _discriminator;

/// <summary>
/// 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.
/// </summary>
public DiscriminatorValueGenerator(object discriminator)
{
_discriminator = discriminator;
}

/// <summary>
/// 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.
/// </summary>
protected override object NextValue(EntityEntry entry)
=> _discriminator;
=> entry.GetInfrastructure().EntityType.GetDiscriminatorValue()!;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"",
Expand Down Expand Up @@ -769,7 +770,6 @@ public static RuntimeEntityType Create(RuntimeModel model, RuntimeEntityType bas
public static void CreateAnnotations(RuntimeEntityType runtimeEntityType)
{
runtimeEntityType.AddAnnotation(""DiscriminatorValue"", ""IdentityUser"");
Customize(runtimeEntityType);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -958,7 +958,8 @@ public static RuntimeEntityType Create(RuntimeModel model, RuntimeEntityType? ba
""Microsoft.EntityFrameworkCore.Scaffolding.Internal.CSharpRuntimeModelCodeGeneratorTest+DependentBase<byte?>"",
typeof(CSharpRuntimeModelCodeGeneratorTest.DependentBase<byte?>),
baseEntityType,
discriminatorProperty: ""EnumDiscriminator"");
discriminatorProperty: ""EnumDiscriminator"",
discriminatorValue: CSharpMigrationsGeneratorTest.Enum1.One);
var principalId = runtimeEntityType.AddProperty(
""PrincipalId"",
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -1480,7 +1480,8 @@ public static RuntimeEntityType Create(RuntimeModel model, RuntimeEntityType? ba
""Microsoft.EntityFrameworkCore.Scaffolding.Internal.CSharpRuntimeModelCodeGeneratorTest+DependentDerived<byte?>"",
typeof(CSharpRuntimeModelCodeGeneratorTest.DependentDerived<byte?>),
baseEntityType,
discriminatorProperty: ""EnumDiscriminator"");
discriminatorProperty: ""EnumDiscriminator"",
discriminatorValue: CSharpMigrationsGeneratorTest.Enum1.Two);
var data = runtimeEntityType.AddProperty(
""Data"",
Expand All @@ -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);
Expand Down
19 changes: 15 additions & 4 deletions test/EFCore.Specification.Tests/CompositeKeyEndToEndTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,24 @@ 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();
}

using (var context = CreateContext())
{
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();
Expand Down Expand Up @@ -212,13 +216,15 @@ public BronieContext(DbContextOptions options)

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.Entity<Pegasus>(
modelBuilder.Entity<Flyer>(
b =>
{
b.HasKey(
e => new { e.Id1, e.Id2 });
e => new { e.Id1, e.Id2, e.Discriminator });
});

modelBuilder.Entity<Pegasus>();

modelBuilder.Entity<Unicorn>(
b =>
{
Expand All @@ -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; }
}

Expand Down

0 comments on commit 3b67c3d

Please sign in to comment.