Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the correct value when the discriminator is part of the key. #26130

Merged
merged 1 commit into from
Sep 22, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
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
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();
}
}
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