Skip to content

Commit

Permalink
Introduce complete discriminator mapping
Browse files Browse the repository at this point in the history
- Add fluent API IsComplete over DiscriminatorBuilder
- In query if mapping is complete
  - Don't generate predicate when creating Select for an entityType
  - Don't apply predicate when doing OfType of derived type when the discriminator is not needed

Current default: Mapping is incomplete unless user uses fluent API to mark it as complete.

Resolves #18106
  • Loading branch information
smitpatel committed Mar 6, 2020
1 parent dee8cbb commit d3a2401
Show file tree
Hide file tree
Showing 22 changed files with 795 additions and 33 deletions.
16 changes: 15 additions & 1 deletion src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -804,9 +804,10 @@ protected virtual void GenerateEntityTypeAnnotations(
}

var discriminatorPropertyAnnotation = annotations.FirstOrDefault(a => a.Name == CoreAnnotationNames.DiscriminatorProperty);
var discriminatorMappingCompleteAnnotation = annotations.FirstOrDefault(a => a.Name == CoreAnnotationNames.DiscriminatorMappingComplete);
var discriminatorValueAnnotation = annotations.FirstOrDefault(a => a.Name == CoreAnnotationNames.DiscriminatorValue);

if ((discriminatorPropertyAnnotation ?? discriminatorValueAnnotation) != null)
if ((discriminatorPropertyAnnotation ?? discriminatorMappingCompleteAnnotation ?? discriminatorValueAnnotation) != null)
{
stringBuilder
.AppendLine()
Expand All @@ -833,6 +834,18 @@ protected virtual void GenerateEntityTypeAnnotations(
.Append("()");
}

if (discriminatorMappingCompleteAnnotation?.Value != null)
{
var value = discriminatorMappingCompleteAnnotation.Value;

stringBuilder
.Append(".")
.Append("IsComplete")
.Append("(")
.Append(Code.UnknownLiteral(value))
.Append(")");
}

if (discriminatorValueAnnotation?.Value != null)
{
var value = discriminatorValueAnnotation.Value;
Expand All @@ -857,6 +870,7 @@ protected virtual void GenerateEntityTypeAnnotations(
stringBuilder.AppendLine(";");

annotations.Remove(discriminatorPropertyAnnotation);
annotations.Remove(discriminatorMappingCompleteAnnotation);
annotations.Remove(discriminatorValueAnnotation);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ protected RelationalQueryableMethodTranslatingExpressionVisitor(

protected override Expression VisitExtension(Expression extensionExpression)
{
switch(extensionExpression)
switch (extensionExpression)
{
case FromSqlQueryRootExpression fromSqlQueryRootExpression:
return CreateShapedQueryExpression(
Expand Down Expand Up @@ -691,37 +691,42 @@ protected override ShapedQueryExpression TranslateOfType(ShapedQueryExpression s
var derivedType = entityType.GetDerivedTypes().SingleOrDefault(et => et.ClrType == resultType);
if (derivedType != null)
{
var selectExpression = (SelectExpression)source.QueryExpression;
var concreteEntityTypes = derivedType.GetConcreteDerivedTypesInclusive().ToList();
var projectionBindingExpression = (ProjectionBindingExpression)entityShaperExpression.ValueBufferExpression;
var entityProjectionExpression = (EntityProjectionExpression)selectExpression.GetMappedProjection(
projectionBindingExpression.ProjectionMember);
var discriminatorColumn = entityProjectionExpression.BindProperty(entityType.GetDiscriminatorProperty());

var predicate = concreteEntityTypes.Count == 1
? _sqlExpressionFactory.Equal(
discriminatorColumn,
_sqlExpressionFactory.Constant(concreteEntityTypes[0].GetDiscriminatorValue()))
: (SqlExpression)_sqlExpressionFactory.In(
discriminatorColumn,
_sqlExpressionFactory.Constant(concreteEntityTypes.Select(et => et.GetDiscriminatorValue())),
negated: false);

selectExpression.ApplyPredicate(predicate);

var projectionMember = projectionBindingExpression.ProjectionMember;

Check.DebugAssert(
new ProjectionMember().Equals(projectionMember),
"Invalid ProjectionMember when processing OfType");

var entityProjection = (EntityProjectionExpression)selectExpression.GetMappedProjection(projectionMember);

selectExpression.ReplaceProjectionMapping(
new Dictionary<ProjectionMember, Expression>
{
if (!derivedType.GetIsDiscriminatorMappingComplete()
|| !derivedType.GetAllBaseTypesInclusiveAscending()
.All(e => (e == derivedType || e.IsAbstract()) && !HasSiblings(e)))
{
var selectExpression = (SelectExpression)source.QueryExpression;
var concreteEntityTypes = derivedType.GetConcreteDerivedTypesInclusive().ToList();
var projectionBindingExpression = (ProjectionBindingExpression)entityShaperExpression.ValueBufferExpression;
var entityProjectionExpression = (EntityProjectionExpression)selectExpression.GetMappedProjection(
projectionBindingExpression.ProjectionMember);
var discriminatorColumn = entityProjectionExpression.BindProperty(entityType.GetDiscriminatorProperty());

var predicate = concreteEntityTypes.Count == 1
? _sqlExpressionFactory.Equal(
discriminatorColumn,
_sqlExpressionFactory.Constant(concreteEntityTypes[0].GetDiscriminatorValue()))
: (SqlExpression)_sqlExpressionFactory.In(
discriminatorColumn,
_sqlExpressionFactory.Constant(concreteEntityTypes.Select(et => et.GetDiscriminatorValue())),
negated: false);

selectExpression.ApplyPredicate(predicate);

var projectionMember = projectionBindingExpression.ProjectionMember;

Check.DebugAssert(
new ProjectionMember().Equals(projectionMember),
"Invalid ProjectionMember when processing OfType");

var entityProjection = (EntityProjectionExpression)selectExpression.GetMappedProjection(projectionMember);

selectExpression.ReplaceProjectionMapping(
new Dictionary<ProjectionMember, Expression>
{
{ projectionMember, entityProjection.UpdateEntityType(derivedType) }
});
});
}

return source.UpdateShaperExpression(entityShaperExpression.WithEntityType(derivedType));
}
Expand All @@ -730,6 +735,11 @@ protected override ShapedQueryExpression TranslateOfType(ShapedQueryExpression s
}

return null;

bool HasSiblings(IEntityType entityType)
{
return entityType.BaseType?.GetDirectlyDerivedTypes().Any(i => i != entityType) == true;
}
}

protected override ShapedQueryExpression TranslateOrderBy(
Expand Down
12 changes: 12 additions & 0 deletions src/EFCore.Relational/Query/SqlExpressionFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -849,6 +849,13 @@ private SqlExpression GenerateJoinPredicate(

private bool AddDiscriminatorCondition(SelectExpression selectExpression, IEntityType entityType)
{
if (entityType.GetIsDiscriminatorMappingComplete()
&& entityType.GetAllBaseTypesInclusiveAscending()
.All(e => (e == entityType || e.IsAbstract()) && !HasSiblings(e)))
{
return false;
}

SqlExpression predicate;
var concreteEntityTypes = entityType.GetConcreteDerivedTypesInclusive().ToList();
if (concreteEntityTypes.Count == 1)
Expand Down Expand Up @@ -876,6 +883,11 @@ private bool AddDiscriminatorCondition(SelectExpression selectExpression, IEntit
selectExpression.ApplyPredicate(predicate);

return true;

bool HasSiblings(IEntityType entityType)
{
return entityType.BaseType?.GetDirectlyDerivedTypes().Any(i => i != entityType) == true;
}
}

private void AddOptionalDependentConditions(
Expand Down
14 changes: 14 additions & 0 deletions src/EFCore/Extensions/ConventionEntityTypeExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -669,6 +669,20 @@ public static void SetDiscriminatorProperty(
=> entityType.FindAnnotation(CoreAnnotationNames.DiscriminatorProperty)
?.GetConfigurationSource();

/// <summary>
/// Sets the value indicating whether the discriminator mapping is complete.
/// </summary>
/// <param name="entityType"> The entity type to set the discriminator mapping complete for. </param>
/// <param name="complete"> The value indicating whether the discriminator mapping is complete. </param>
/// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
public static void SetDiscriminatorMappingComplete(
[NotNull] this IConventionEntityType entityType, bool? complete, bool fromDataAnnotation = false)
{
Check.NotNull(entityType, nameof(entityType));

entityType.GetRootType().SetOrRemoveAnnotation(CoreAnnotationNames.DiscriminatorMappingComplete, complete, fromDataAnnotation);
}

/// <summary>
/// Sets the discriminator value for this entity type.
/// </summary>
Expand Down
14 changes: 14 additions & 0 deletions src/EFCore/Extensions/EntityTypeExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -647,6 +647,20 @@ public static IProperty GetDiscriminatorProperty([NotNull] this IEntityType enti
return propertyName == null ? null : entityType.FindProperty(propertyName);
}

/// <summary>
/// Returns the value indicating whether the discriminator mapping is complete for this entity type.
/// </summary>
/// <param name="entityType"> The entity type to check whether the discriminator mapping is complete. </param>
public static bool GetIsDiscriminatorMappingComplete([NotNull] this IEntityType entityType)
{
if (entityType.BaseType != null)
{
return entityType.GetRootType().GetIsDiscriminatorMappingComplete();
}

return (bool?)entityType[CoreAnnotationNames.DiscriminatorMappingComplete] ?? false;
}

/// <summary>
/// Returns the discriminator value for this entity type.
/// </summary>
Expand Down
12 changes: 12 additions & 0 deletions src/EFCore/Extensions/MutableEntityTypeExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -571,6 +571,18 @@ public static void SetDiscriminatorProperty([NotNull] this IMutableEntityType en
=> Check.NotNull(entityType, nameof(entityType)).AsEntityType()
.SetDiscriminatorProperty(property, ConfigurationSource.Explicit);

/// <summary>
/// Sets the value indicating whether the discriminator mapping is complete.
/// </summary>
/// <param name="entityType"> The entity type to set the discriminator mapping complete for. </param>
/// <param name="complete"> The value indicating whether the discriminator mapping is complete. </param>
public static void SetDiscriminatorMappingComplete([NotNull] this IMutableEntityType entityType, bool? complete)
{
Check.NotNull(entityType, nameof(entityType));

entityType.GetRootType().SetOrRemoveAnnotation(CoreAnnotationNames.DiscriminatorMappingComplete, complete);
}

/// <summary>
/// Sets the discriminator value for this entity type.
/// </summary>
Expand Down
37 changes: 37 additions & 0 deletions src/EFCore/Metadata/Builders/DiscriminatorBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,34 @@ public DiscriminatorBuilder([NotNull] IMutableEntityType entityType)
[EntityFrameworkInternal]
protected virtual InternalEntityTypeBuilder EntityTypeBuilder { get; }

/// <summary>
/// Configures if the discriminator mapping is complete.
/// </summary>
/// <param name="complete"> The value indicating if this discriminator mapping is complete. </param>
/// <returns> The same builder so that multiple calls can be chained. </returns>
public virtual DiscriminatorBuilder IsComplete(bool complete = true)
=> IsComplete(complete, ConfigurationSource.Explicit);

private DiscriminatorBuilder IsComplete(bool complete, ConfigurationSource configurationSource)
{
if (configurationSource == ConfigurationSource.Explicit)
{
EntityTypeBuilder.Metadata.SetDiscriminatorMappingComplete(complete);
}
else
{
if (!EntityTypeBuilder.CanSetAnnotation(
CoreAnnotationNames.DiscriminatorMappingComplete, complete, configurationSource))
{
return null;
}

EntityTypeBuilder.Metadata.SetDiscriminatorValue(complete, configurationSource == ConfigurationSource.DataAnnotation);
}

return this;
}

/// <summary>
/// Configures the default discriminator value to use.
/// </summary>
Expand Down Expand Up @@ -119,6 +147,15 @@ private DiscriminatorBuilder HasValue(
return this;
}

/// <inheritdoc />
IConventionDiscriminatorBuilder IConventionDiscriminatorBuilder.IsComplete(bool complete, bool fromDataAnnotation)
=> IsComplete(complete, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention);

/// <inheritdoc />
bool IConventionDiscriminatorBuilder.CanSetIsComplete(bool complete, bool fromDataAnnotation)
=> ((IConventionEntityTypeBuilder)EntityTypeBuilder).CanSetAnnotation(
CoreAnnotationNames.DiscriminatorMappingComplete, fromDataAnnotation);

/// <inheritdoc />
IConventionDiscriminatorBuilder IConventionDiscriminatorBuilder.HasValue(object value, bool fromDataAnnotation)
=> HasValue(
Expand Down
11 changes: 11 additions & 0 deletions src/EFCore/Metadata/Builders/DiscriminatorBuilder`.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,17 @@ public DiscriminatorBuilder([NotNull] DiscriminatorBuilder builder)

private DiscriminatorBuilder Builder { get; }

/// <summary>
/// Configures if the discriminator mapping is complete.
/// </summary>
/// <param name="complete"> The value indicating if this discriminator mapping is complete. </param>
/// <returns> The same builder so that multiple calls can be chained. </returns>
public virtual DiscriminatorBuilder<TDiscriminator> IsComplete(bool complete = true)
{
var builder = Builder.IsComplete(complete);
return builder == null ? null : new DiscriminatorBuilder<TDiscriminator>(builder);
}

/// <summary>
/// Configures the default discriminator value to use.
/// </summary>
Expand Down
16 changes: 16 additions & 0 deletions src/EFCore/Metadata/Builders/IConventionDiscriminatorBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,22 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Builders
/// </summary>
public interface IConventionDiscriminatorBuilder
{
/// <summary>
/// Configures if the discriminator mapping is complete.
/// </summary>
/// <param name="complete"> The value indicating if this discriminator mapping is complete. </param>
/// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
/// <returns> The same builder so that multiple calls can be chained. </returns>
IConventionDiscriminatorBuilder IsComplete(bool complete, bool fromDataAnnotation = false);

/// <summary>
/// Returns a value indicating whether the discriminator mapping is complete can be set from this configuration source.
/// </summary>
/// <param name="complete"> The value indicating if this discriminator mapping is complete. </param>
/// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
/// <returns> <c>true</c> if the discriminator value can be set from this configuration source. </returns>
bool CanSetIsComplete(bool complete, bool fromDataAnnotation = false);

/// <summary>
/// Configures the discriminator value to use.
/// </summary>
Expand Down
9 changes: 9 additions & 0 deletions src/EFCore/Metadata/Internal/CoreAnnotationNames.cs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,14 @@ public static class CoreAnnotationNames
/// </summary>
public const string DiscriminatorProperty = "DiscriminatorProperty";

/// <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 const string DiscriminatorMappingComplete = "DiscriminatorMappingComplete";

/// <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
Expand Down Expand Up @@ -248,6 +256,7 @@ public static class CoreAnnotationNames
ChangeTrackingStrategy,
OwnedTypes,
DiscriminatorProperty,
DiscriminatorMappingComplete,
DiscriminatorValue,
ConstructorBinding,
TypeMapping,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,7 @@ public virtual void Discriminator_annotations_are_stored_in_snapshot()
builder.Entity<AnotherDerivedEntity>().HasBaseType<BaseEntity>();
builder.Entity<BaseEntity>()
.HasDiscriminator(e => e.Discriminator)
.IsComplete()
.HasValue(typeof(BaseEntity), typeof(BaseEntity).Name)
.HasValue(typeof(DerivedEntity), typeof(DerivedEntity).Name)
.HasValue(typeof(AnotherDerivedEntity), typeof(AnotherDerivedEntity).Name);
Expand All @@ -573,7 +574,7 @@ public virtual void Discriminator_annotations_are_stored_in_snapshot()
b.ToTable(""BaseEntity"");
b.HasDiscriminator<string>(""Discriminator"").HasValue(""BaseEntity"");
b.HasDiscriminator<string>(""Discriminator"").IsComplete(true).HasValue(""BaseEntity"");
});
modelBuilder.Entity(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+AnotherDerivedEntity"", b =>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

namespace Microsoft.EntityFrameworkCore.Query
{
public class CompleteMappingInheritanceInMemoryFixture : InheritanceInMemoryFixture
{
protected override bool IsDiscriminatorMappingComplete => true;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Xunit;

namespace Microsoft.EntityFrameworkCore.Query
{
public class CompleteMappingInheritanceInMemoryTest : InheritanceTestBase<CompleteMappingInheritanceInMemoryFixture>
{
public CompleteMappingInheritanceInMemoryTest(CompleteMappingInheritanceInMemoryFixture fixture)
: base(fixture)
{
}

[ConditionalFact]
public override void Can_query_all_animal_views()
{
var message = Assert.Throws<InvalidOperationException>(() => base.Can_query_all_animal_views()).Message;

Assert.Equal(
CoreStrings.TranslationFailed(
@"DbSet<Bird>()
.Select(b => InheritanceInMemoryFixture.MaterializeView(b))
.OrderBy(a => a.CountryId)"),
message);
}

protected override bool EnforcesFkConstraints => false;
}
}
Loading

0 comments on commit d3a2401

Please sign in to comment.