Query: Use GetDiscriminatorValue for TPT and TPC (#28070)

Add model validation when values are not unique
Resolves #28054

src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs

@@ -1391,6 +1391,30 @@ protected override void ValidateInheritanceMapping(
 
                 ValidateNonTphMapping(entityType, forTables: false);
                 ValidateNonTphMapping(entityType, forTables: true);
+
+                var derivedTypes = entityType.GetDerivedTypesInclusive().ToList();
+                var discriminatorValues = new Dictionary<string, IEntityType>();
+                foreach (var derivedType in derivedTypes)
+                {
+                    if (!derivedType.ClrType.IsInstantiable())
+                    {
+                        continue;
+                    }
+                    var discriminatorValue = derivedType.GetDiscriminatorValue();
+                    if (discriminatorValue is not string valueString)
+                    {
+                        throw new InvalidOperationException(
+                            RelationalStrings.NonTphDiscriminatorValueNotString(discriminatorValue, derivedType.DisplayName()));
+                    }
+
+                    if (discriminatorValues.TryGetValue(valueString, out var duplicateEntityType))
+                    {
+                        throw new InvalidOperationException(RelationalStrings.EntityShortNameNotUnique(
+                            derivedType.Name, discriminatorValue, duplicateEntityType.Name));
+                    }
+
+                    discriminatorValues[valueString] = derivedType;
+                }
             }
         }
 
@@ -1469,7 +1493,7 @@ private static void ValidateNonTphMapping(IEntityType rootEntityType, bool forTa
         {
             return;
         }
-        
+
         var internalForeignKey = rootEntityType.FindRowInternalForeignKeys(storeObject.Value).FirstOrDefault();
         if (internalForeignKey != null
             && derivedTypes.Count > 1

src/EFCore.Relational/Metadata/Internal/RelationalEntityTypeExtensions.cs

@@ -31,15 +31,6 @@ public static IEnumerable<ITableMappingBase> GetViewOrTableMappings(this IEntity
                 ?? entityType.FindRuntimeAnnotationValue(RelationalAnnotationNames.TableMappings))
             ?? Enumerable.Empty<ITableMappingBase>();
 
-    /// <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 static IReadOnlyList<string> GetTptDiscriminatorValues(this IReadOnlyEntityType entityType)
-        => entityType.GetConcreteDerivedTypesInclusive().Select(et => et.ShortName()).ToList();
-
     /// <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

src/EFCore.Relational/Properties/RelationalStrings.resx

@@ -340,6 +340,9 @@
   <data name="EitherOfTwoValuesMustBeNull" xml:space="preserve">
     <value>Either {param1} or {param2} must be null.</value>
   </data>
+  <data name="EntityShortNameNotUnique" xml:space="preserve">
+    <value>The short name for '{entityType1}' is '{discriminatorValue}' which is the same for '{entityType2}'. Every concrete entity type in the hierarchy must have a unique short name. Either rename one of the types or call modelBuilder.Entity&lt;TEntity&gt;().Metadata.SetDiscriminatorValue("NewShortName").</value>
+  </data>
   <data name="ErrorMaterializingProperty" xml:space="preserve">
     <value>An error occurred while reading a database value for property '{entityType}.{property}'. See the inner exception for more information.</value>
   </data>
@@ -763,6 +766,9 @@
   <data name="NonScalarFunctionParameterCannotPropagatesNullability" xml:space="preserve">
     <value>Cannot set 'PropagatesNullability' on parameter '{parameterName}' of DbFunction '{functionName}' since function does not represent a scalar function.</value>
   </data>
+  <data name="NonTphDiscriminatorValueNotString" xml:space="preserve">
+    <value>The specified discriminator value '{value}' for '{entityType}' is not a string. Configure a string discriminator value instead.</value>
+  </data>
   <data name="NonTphMappingStrategy" xml:space="preserve">
     <value>The mapping strategy '{mappingStrategy}' specified on '{entityType}' is not supported for entity types with a discriminator.</value>
   </data>

src/EFCore.Relational/Query/EntityProjectionExpression.cs

@@ -123,7 +123,7 @@ public virtual EntityProjectionExpression UpdateEntityType(IEntityType derivedTy
         var discriminatorExpression = DiscriminatorExpression;
         if (DiscriminatorExpression is CaseExpression caseExpression)
         {
-            var entityTypesToSelect = derivedType.GetTptDiscriminatorValues();
+            var entityTypesToSelect = derivedType.GetConcreteDerivedTypesInclusive().Select(e => e.GetDiscriminatorValue()).ToList();
             var whenClauses = caseExpression.WhenClauses
                 .Where(wc => entityTypesToSelect.Contains((string)((SqlConstantExpression)wc.Result).Value!))
                 .ToList();

src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs

@@ -1128,7 +1128,7 @@ protected override Expression VisitTypeBinary(TypeBinaryExpression typeBinaryExp
                     }
 
                     // TPT or TPC
-                    var discriminatorValues = derivedType.GetTptDiscriminatorValues();
+                    var discriminatorValues = derivedType.GetConcreteDerivedTypesInclusive().Select(e => e.GetDiscriminatorValue()).ToList();
                     if (entityReferenceExpression.SubqueryEntity != null)
                     {
                         var entityShaper = (EntityShaperExpression)entityReferenceExpression.SubqueryEntity.ShaperExpression;

src/EFCore/Infrastructure/ConventionAnnotatable.cs

@@ -133,9 +133,16 @@ public override void SetAnnotation(string name, object? value)
     ///     Removes the given annotation from this object.
     /// </summary>
     /// <param name="name">The annotation to remove.</param>
+    /// <param name="configurationSource">The configuration source of the annotation to be removed.</param>
     /// <returns>The annotation that was removed.</returns>
-    public new virtual ConventionAnnotation? RemoveAnnotation(string name)
-        => (ConventionAnnotation?)base.RemoveAnnotation(name);
+    public virtual ConventionAnnotation? RemoveAnnotation(string name, ConfigurationSource configurationSource)
+    {
+        var annotation = FindAnnotation(name);
+        return annotation == null
+            || !configurationSource.Overrides(annotation.GetConfigurationSource())
+            ? null
+            : (ConventionAnnotation?)base.RemoveAnnotation(name);
+    }
 
     /// <inheritdoc />
     protected override Annotation CreateAnnotation(string name, object? value)
@@ -201,8 +208,8 @@ IConventionAnnotation IConventionAnnotatable.AddAnnotation(string name, object?
 
     /// <inheritdoc />
     [DebuggerStepThrough]
-    IConventionAnnotation? IConventionAnnotatable.RemoveAnnotation(string name)
-        => RemoveAnnotation(name);
+    IConventionAnnotation? IConventionAnnotatable.RemoveAnnotation(string name, bool fromDataAnnotation)
+        => RemoveAnnotation(name, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention);
 
     /// <inheritdoc />
     [DebuggerStepThrough]

src/EFCore/Infrastructure/ModelValidator.cs

@@ -604,13 +604,19 @@ protected virtual void ValidateDiscriminatorValues(IEntityType rootEntityType)
                 continue;
             }
 
-            var discriminatorValue = derivedType.GetDiscriminatorValue();
+            var discriminatorValue = derivedType[CoreAnnotationNames.DiscriminatorValue];
             if (discriminatorValue == null)
             {
                 throw new InvalidOperationException(
                     CoreStrings.NoDiscriminatorValue(derivedType.DisplayName()));
             }
 
+            if (!discriminatorProperty.ClrType.IsInstanceOfType(discriminatorValue))
+            {
+                throw new InvalidOperationException(
+                    CoreStrings.DiscriminatorValueIncompatible(discriminatorValue, discriminatorProperty.Name, discriminatorProperty.ClrType));
+            }
+
             if (discriminatorValues.TryGetValue(discriminatorValue, out var duplicateEntityType))
             {
                 throw new InvalidOperationException(

src/EFCore/Metadata/IConventionAnnotatable.cs

@@ -66,8 +66,9 @@ public interface IConventionAnnotatable : IReadOnlyAnnotatable
     ///     Removes the annotation with the given name from this object.
     /// </summary>
     /// <param name="name">The name of the annotation to remove.</param>
+    /// <param name="fromDataAnnotation">Indicates whether the configuration was specified using a data annotation.</param>
     /// <returns>The annotation that was removed.</returns>
-    IConventionAnnotation? RemoveAnnotation(string name);
+    IConventionAnnotation? RemoveAnnotation(string name, bool fromDataAnnotation = false);
 
     /// <summary>
     ///     Gets the annotation with the given name, throwing if it does not exist.

src/EFCore/Metadata/IConventionEntityType.cs

@@ -120,15 +120,16 @@ public interface IConventionEntityType : IReadOnlyEntityType, IConventionTypeBas
     /// <param name="fromDataAnnotation">Indicates whether the configuration was specified using a data annotation.</param>
     /// <returns>The configured value.</returns>
     object? SetDiscriminatorValue(object? value, bool fromDataAnnotation = false)
-        => SetAnnotation(CoreAnnotationNames.DiscriminatorValue, EntityType.CheckDiscriminatorValue(this, value), fromDataAnnotation)
+        => SetAnnotation(CoreAnnotationNames.DiscriminatorValue, value, fromDataAnnotation)
             ?.Value;
 
     /// <summary>
     ///     Removes the discriminator value for this entity type.
     /// </summary>
+    /// <param name="fromDataAnnotation">Indicates whether the configuration was specified using a data annotation.</param>
     /// <returns>The removed discriminator value.</returns>
-    object? RemoveDiscriminatorValue()
-        => RemoveAnnotation(CoreAnnotationNames.DiscriminatorValue)?.Value;
+    object? RemoveDiscriminatorValue(bool fromDataAnnotation = false)
+        => RemoveAnnotation(CoreAnnotationNames.DiscriminatorValue, fromDataAnnotation)?.Value;
 
     /// <summary>
     ///     Gets the <see cref="ConfigurationSource" /> for the discriminator value.

src/EFCore/Metadata/IMutableEntityType.cs

@@ -80,7 +80,7 @@ void SetDiscriminatorMappingComplete(bool? complete)
     /// </summary>
     /// <param name="value">The value to set.</param>
     void SetDiscriminatorValue(object? value)
-        => SetAnnotation(CoreAnnotationNames.DiscriminatorValue, EntityType.CheckDiscriminatorValue(this, value));
+        => SetAnnotation(CoreAnnotationNames.DiscriminatorValue, value);
 
     /// <summary>
     ///     Removes the discriminator value for this entity type.

src/EFCore/Metadata/IReadOnlyEntityType.cs

@@ -70,7 +70,15 @@ bool GetIsDiscriminatorMappingComplete()
     /// </summary>
     /// <returns>The discriminator value for this entity type.</returns>
     object? GetDiscriminatorValue()
-        => this[CoreAnnotationNames.DiscriminatorValue];
+    {
+        var annotation = FindAnnotation(CoreAnnotationNames.DiscriminatorValue);
+        return annotation != null
+            ? annotation.Value
+            : !ClrType.IsInstantiable()
+                || (BaseType == null && GetDirectlyDerivedTypes().Count() == 0)
+            ? null
+            : (object)ShortName();
+    }
 
     /// <summary>
     ///     Gets all types in the model from which a given entity type derives, starting with the root.

src/EFCore/Metadata/Internal/EntityType.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.ComponentModel;
+using System.Runtime.CompilerServices;
 using Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
 using Microsoft.EntityFrameworkCore.Internal;
 
@@ -3032,7 +3033,7 @@ public virtual IEnumerable<ServiceProperty> GetDerivedServiceProperties()
         {
             return Enumerable.Empty<IDictionary<string, object?>>();
         }
-        
+
         List<IPropertyBase>? propertiesList = null;
         Dictionary<string, IPropertyBase>? propertiesMap = null;
         var data = new List<Dictionary<string, object?>>();
@@ -3056,7 +3057,7 @@ public virtual IEnumerable<ServiceProperty> GetDerivedServiceProperties()
                     {
                         continue;
                     }
-                    
+
                     ValueConverter? valueConverter = null;
                     if (providerValues
                         && propertyBase is IProperty property
@@ -3347,12 +3348,12 @@ public virtual ChangeTrackingStrategy GetChangeTrackingStrategy()
         if (((property == null && BaseType == null)
                 || (property != null && !property.ClrType.IsInstanceOfType(((IReadOnlyEntityType)this).GetDiscriminatorValue()))))
         {
-            ((IMutableEntityType)this).RemoveDiscriminatorValue();
+            RemoveDiscriminatorValue(this, configurationSource);
             if (BaseType == null)
             {
                 foreach (var derivedType in GetDerivedTypes())
                 {
-                    ((IMutableEntityType)derivedType).RemoveDiscriminatorValue();
+                    RemoveDiscriminatorValue(derivedType, configurationSource);
                 }
             }
         }
@@ -3361,6 +3362,18 @@ public virtual ChangeTrackingStrategy GetChangeTrackingStrategy()
             == property?.Name
                 ? property
                 : (Property?)((IReadOnlyEntityType)this).FindDiscriminatorProperty();
+
+        static void RemoveDiscriminatorValue(IReadOnlyEntityType entityType, ConfigurationSource configurationSource)
+        {
+            if (configurationSource is ConfigurationSource.Convention or ConfigurationSource.DataAnnotation)
+            {
+                ((IConventionEntityType)entityType).RemoveDiscriminatorValue(configurationSource == ConfigurationSource.DataAnnotation);
+            }
+            else
+            {
+                ((IMutableEntityType)entityType).RemoveDiscriminatorValue();
+            }
+        }
     }
 
     private void CheckDiscriminatorProperty(Property? property)
@@ -3395,35 +3408,6 @@ private void CheckDiscriminatorProperty(Property? property)
         return (string?)this[CoreAnnotationNames.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 static object? CheckDiscriminatorValue(IReadOnlyEntityType entityType, object? value)
-    {
-        if (value is null)
-        {
-            return value;
-        }
-
-        var discriminatorProperty = entityType.FindDiscriminatorProperty();
-        if (discriminatorProperty is null)
-        {
-            throw new InvalidOperationException(
-                CoreStrings.NoDiscriminatorForValue(entityType.DisplayName(), entityType.GetRootType().DisplayName()));
-        }
-
-        if (!discriminatorProperty.ClrType.IsInstanceOfType(value))
-        {
-            throw new InvalidOperationException(
-                CoreStrings.DiscriminatorValueIncompatible(value, discriminatorProperty.Name, discriminatorProperty.ClrType));
-        }
-
-        return value;
-    }
-
     /// <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