Skip to content

Commit

Permalink
Determine column names for TPT in the model instead of a convention.
Browse files Browse the repository at this point in the history 
Fixes #21961
  • Loading branch information
@AndriySvyryd
AndriySvyryd committed Aug 12, 2020
1 parent f69b5ac commit 59e1277
Show file tree
Hide file tree
Showing 8 changed files with 250 additions and 65 deletions.
8 changes: 8 additions & 0 deletions src/EFCore.Relational/Extensions/RelationalPropertyExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,14 @@ public static string GetColumnName([NotNull] this IProperty property, in StoreOb
return overrides.ColumnName;
}

if (!property.IsPrimaryKey()
&& storeObject.StoreObjectType != StoreObjectType.Function
&& storeObject.StoreObjectType != StoreObjectType.SqlQuery
&& StoreObjectIdentifier.Create(property.DeclaringEntityType, storeObject.StoreObjectType) != storeObject)
{
return null;
}

var columnAnnotation = property.FindAnnotation(RelationalAnnotationNames.ColumnName);
if (columnAnnotation != null)
{
Expand Down
18 changes: 0 additions & 18 deletions src/EFCore.Relational/Metadata/Conventions/EntityTypeHierarchyMappingConvention.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,6 @@ public virtual void ProcessModelFinalizing(IConventionModelBuilder modelBuilder,
|| schema != entityType.BaseType.GetSchema()))
{
nonTphRoots.Add(entityType.GetRootType());
foreach (var property in entityType.BaseType.GetProperties())
{
if (property.IsPrimaryKey())
{
continue;
}

property.Builder.HasColumnName(null, StoreObjectIdentifier.Table(tableName, schema));
}
}

var viewName = entityType.GetViewName();
Expand All @@ -61,15 +52,6 @@ public virtual void ProcessModelFinalizing(IConventionModelBuilder modelBuilder,
|| viewSchema != entityType.BaseType.GetViewSchema()))
{
nonTphRoots.Add(entityType.GetRootType());
foreach (var property in entityType.BaseType.GetProperties())
{
if (property.IsPrimaryKey())
{
continue;
}

property.Builder.HasColumnName(null, StoreObjectIdentifier.View(viewName, viewSchema));
}
}
}

Expand Down
31 changes: 11 additions & 20 deletions src/EFCore.Relational/Metadata/Internal/RelationalForeignKeyExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,10 @@ public static bool AreCompatible(
bool shouldThrow)
{
var principalType = foreignKey.PrincipalEntityType;
var principalTable = StoreObjectIdentifier.Create(principalType, StoreObjectType.Table);
var duplicatePrincipalType = duplicateForeignKey.PrincipalEntityType;
if (!string.Equals(principalType.GetSchema(), duplicatePrincipalType.GetSchema(), StringComparison.OrdinalIgnoreCase)
|| !string.Equals(principalType.GetTableName(), duplicatePrincipalType.GetTableName(), StringComparison.OrdinalIgnoreCase))
var duplicatePrincipalTable = StoreObjectIdentifier.Create(duplicatePrincipalType, StoreObjectType.Table);
if (principalTable != duplicatePrincipalTable)
{
if (shouldThrow)
{
Expand All @@ -44,9 +45,7 @@ public static bool AreCompatible(
duplicateForeignKey.Properties.Format(),
duplicateForeignKey.DeclaringEntityType.DisplayName(),
foreignKey.DeclaringEntityType.GetSchemaQualifiedTableName(),
foreignKey.GetConstraintName(storeObject,
StoreObjectIdentifier.Table(foreignKey.PrincipalEntityType.GetTableName(),
foreignKey.PrincipalEntityType.GetSchema())),
foreignKey.GetConstraintName(storeObject, principalTable.Value),
principalType.GetSchemaQualifiedTableName(),
duplicatePrincipalType.GetSchemaQualifiedTableName()));
}
Expand All @@ -65,17 +64,15 @@ public static bool AreCompatible(
duplicateForeignKey.Properties.Format(),
duplicateForeignKey.DeclaringEntityType.DisplayName(),
foreignKey.DeclaringEntityType.GetSchemaQualifiedTableName(),
foreignKey.GetConstraintName(storeObject,
StoreObjectIdentifier.Table(foreignKey.PrincipalEntityType.GetTableName(),
foreignKey.PrincipalEntityType.GetSchema())),
foreignKey.GetConstraintName(storeObject, principalTable.Value),
foreignKey.Properties.FormatColumns(storeObject),
duplicateForeignKey.Properties.FormatColumns(storeObject)));
}

return false;
}

if (!SameColumnNames(foreignKey.PrincipalKey.Properties, duplicateForeignKey.PrincipalKey.Properties, storeObject))
if (!SameColumnNames(foreignKey.PrincipalKey.Properties, duplicateForeignKey.PrincipalKey.Properties, principalTable.Value))
{
if (shouldThrow)
{
Expand All @@ -86,11 +83,9 @@ public static bool AreCompatible(
duplicateForeignKey.Properties.Format(),
duplicateForeignKey.DeclaringEntityType.DisplayName(),
foreignKey.DeclaringEntityType.GetSchemaQualifiedTableName(),
foreignKey.GetConstraintName(storeObject,
StoreObjectIdentifier.Table(foreignKey.PrincipalEntityType.GetTableName(),
foreignKey.PrincipalEntityType.GetSchema())),
foreignKey.PrincipalKey.Properties.FormatColumns(storeObject),
duplicateForeignKey.PrincipalKey.Properties.FormatColumns(storeObject)));
foreignKey.GetConstraintName(storeObject, principalTable.Value),
foreignKey.PrincipalKey.Properties.FormatColumns(principalTable.Value),
duplicateForeignKey.PrincipalKey.Properties.FormatColumns(principalTable.Value)));
}

return false;
Expand All @@ -107,9 +102,7 @@ public static bool AreCompatible(
duplicateForeignKey.Properties.Format(),
duplicateForeignKey.DeclaringEntityType.DisplayName(),
foreignKey.DeclaringEntityType.GetSchemaQualifiedTableName(),
foreignKey.GetConstraintName(storeObject,
StoreObjectIdentifier.Table(foreignKey.PrincipalEntityType.GetTableName(),
foreignKey.PrincipalEntityType.GetSchema()))));
foreignKey.GetConstraintName(storeObject, principalTable.Value)));
}

return false;
Expand All @@ -126,9 +119,7 @@ public static bool AreCompatible(
duplicateForeignKey.Properties.Format(),
duplicateForeignKey.DeclaringEntityType.DisplayName(),
foreignKey.DeclaringEntityType.GetSchemaQualifiedTableName(),
foreignKey.GetConstraintName(storeObject,
StoreObjectIdentifier.Table(foreignKey.PrincipalEntityType.GetTableName(),
foreignKey.PrincipalEntityType.GetSchema())),
foreignKey.GetConstraintName(storeObject, principalTable.Value),
foreignKey.DeleteBehavior,
duplicateForeignKey.DeleteBehavior));
}
Expand Down
63 changes: 62 additions & 1 deletion src/EFCore.Relational/Metadata/StoreObjectIdentifier.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Microsoft.EntityFrameworkCore.Metadata
/// <summary>
/// A type that represents the id of a store object
/// </summary>
public readonly struct StoreObjectIdentifier : IComparable<StoreObjectIdentifier>
public readonly struct StoreObjectIdentifier : IComparable<StoreObjectIdentifier>, IEquatable<StoreObjectIdentifier>
{
private StoreObjectIdentifier(StoreObjectType storeObjectType, string name, string schema = null)
{
Expand All @@ -19,6 +19,35 @@ private StoreObjectIdentifier(StoreObjectType storeObjectType, string name, stri
Schema = schema;
}

/// <summary>
/// Creates an id for the store object that the given entity type is mapped to />.
/// </summary>
/// <param name="entityType"> The entity type. </param>
/// <param name="type"> The store object type. </param>
/// <returns> The store object id. </returns>
public static StoreObjectIdentifier? Create([NotNull] IEntityType entityType, StoreObjectType type)
{
Check.NotNull(entityType, nameof(entityType));

switch (type)
{
case StoreObjectType.Table:
var tableName = entityType.GetTableName();
return tableName == null ? (StoreObjectIdentifier?)null : Table(tableName, entityType.GetSchema());
case StoreObjectType.View:
var viewName = entityType.GetViewName();
return viewName == null ? (StoreObjectIdentifier?)null : View(viewName, entityType.GetViewSchema());
case StoreObjectType.SqlQuery:
var query = entityType.GetSqlQuery();
return query == null ? (StoreObjectIdentifier?)null : SqlQuery(entityType);
case StoreObjectType.Function:
var functionName = entityType.GetFunctionName();
return functionName == null ? (StoreObjectIdentifier?)null : DbFunction(functionName);
default:
return null;
}
}

/// <summary>
/// Creates a table id.
/// </summary>
Expand Down Expand Up @@ -121,5 +150,37 @@ public int CompareTo(StoreObjectIdentifier other)

/// <inheritdoc />
public override string ToString() => StoreObjectType.ToString() + " " + DisplayName();

/// <inheritdoc />
public override bool Equals(object obj)
=> obj is StoreObjectIdentifier identifier && Equals(identifier);

/// <inheritdoc />
public bool Equals(StoreObjectIdentifier other)
=> StoreObjectType == other.StoreObjectType &&
Name == other.Name &&
Schema == other.Schema;

/// <inheritdoc />
public override int GetHashCode()
=> HashCode.Combine(StoreObjectType, Name, Schema);

/// <summary>
/// Compares one id to another id to see if they represent the same store object.
/// </summary>
/// <param name="left"> The first id. </param>
/// <param name="right"> The second id. </param>
/// <returns> <see langword="true"/> if they represent the same store object; <see langword="false"/> otherwise. </returns>
public static bool operator ==(StoreObjectIdentifier left, StoreObjectIdentifier right)
=> left.Equals(right);

/// <summary>
/// Compares one id to another id to see if they represent the same store object.
/// </summary>
/// <param name="left"> The first id. </param>
/// <param name="right"> The second id. </param>
/// <returns> <see langword="false"/> if they represent the same store object; <see langword="true"/> otherwise. </returns>
public static bool operator !=(StoreObjectIdentifier left, StoreObjectIdentifier right)
=> !(left == right);
}
}
24 changes: 12 additions & 12 deletions src/EFCore/Diagnostics/CoreEventId.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ private enum Id
ShadowPropertyCreated = CoreBaseId + 600,
RedundantIndexRemoved,
IncompatibleMatchingForeignKeyProperties,
RequiredAttributeOnDependent, // Unused
RequiredAttributeOnBothNavigations, // Unused
Obsolete_RequiredAttributeOnDependent,
Obsolete_RequiredAttributeOnBothNavigations,
ConflictingShadowForeignKeysWarning,
MultiplePrimaryKeyCandidates,
MultipleNavigationProperties,
Expand All @@ -100,10 +100,10 @@ private enum Id
ForeignKeyAttributesOnBothNavigationsWarning,
ConflictingForeignKeyAttributesOnNavigationAndPropertyWarning,
RedundantForeignKeyWarning,
NonNullableInverted, // Unused
NonNullableReferenceOnBothNavigations, // Unused
NonNullableReferenceOnDependent, // Unused
RequiredAttributeInverted, // Unused
Obsolete_NonNullableInverted,
Obsolete_NonNullableReferenceOnBothNavigations,
Obsolete_NonNullableReferenceOnDependent,
Obsolete_RequiredAttributeInverted,
RequiredAttributeOnCollection,
CollectionWithoutComparer,
ConflictingKeylessAndKeyAttributesWarning,
Expand Down Expand Up @@ -475,7 +475,7 @@ public static readonly EventId FirstWithoutOrderByAndFilterWarning
/// </para>
/// </summary>
[Obsolete]
public static readonly EventId RequiredAttributeInverted = MakeModelId(Id.RequiredAttributeInverted);
public static readonly EventId RequiredAttributeInverted = MakeModelId(Id.Obsolete_RequiredAttributeInverted);

/// <summary>
/// <para>
Expand All @@ -490,7 +490,7 @@ public static readonly EventId FirstWithoutOrderByAndFilterWarning
/// </para>
/// </summary>
[Obsolete]
public static readonly EventId NonNullableInverted = MakeModelId(Id.NonNullableInverted);
public static readonly EventId NonNullableInverted = MakeModelId(Id.Obsolete_NonNullableInverted);

/// <summary>
/// <para>
Expand All @@ -504,7 +504,7 @@ public static readonly EventId FirstWithoutOrderByAndFilterWarning
/// </para>
/// </summary>
[Obsolete]
public static readonly EventId RequiredAttributeOnBothNavigations = MakeModelId(Id.RequiredAttributeOnBothNavigations);
public static readonly EventId RequiredAttributeOnBothNavigations = MakeModelId(Id.Obsolete_RequiredAttributeOnBothNavigations);

/// <summary>
/// <para>
Expand All @@ -518,7 +518,7 @@ public static readonly EventId FirstWithoutOrderByAndFilterWarning
/// </para>
/// </summary>
[Obsolete]
public static readonly EventId NonNullableReferenceOnBothNavigations = MakeModelId(Id.NonNullableReferenceOnBothNavigations);
public static readonly EventId NonNullableReferenceOnBothNavigations = MakeModelId(Id.Obsolete_NonNullableReferenceOnBothNavigations);

/// <summary>
/// <para>
Expand All @@ -532,7 +532,7 @@ public static readonly EventId FirstWithoutOrderByAndFilterWarning
/// </para>
/// </summary>
[Obsolete]
public static readonly EventId RequiredAttributeOnDependent = MakeModelId(Id.RequiredAttributeOnDependent);
public static readonly EventId RequiredAttributeOnDependent = MakeModelId(Id.Obsolete_RequiredAttributeOnDependent);

/// <summary>
/// <para>
Expand All @@ -546,7 +546,7 @@ public static readonly EventId FirstWithoutOrderByAndFilterWarning
/// </para>
/// </summary>
[Obsolete]
public static readonly EventId NonNullableReferenceOnDependent = MakeModelId(Id.NonNullableReferenceOnDependent);
public static readonly EventId NonNullableReferenceOnDependent = MakeModelId(Id.Obsolete_NonNullableReferenceOnDependent);

/// <summary>
/// <para>
Expand Down
16 changes: 8 additions & 8 deletions test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,27 +147,27 @@ protected virtual ModelBuilder CreateModelBuilder(bool skipConventions)
private ConventionSet CreateEmptyConventionSet()
{
var conventions = new ConventionSet();
var conventionSetDependencies = TestHelpers.CreateContextServices().GetRequiredService<ProviderConventionSetBuilderDependencies>();
var relationalConventionSetDependencies = new RelationalConventionSetBuilderDependencies(
new RelationalAnnotationProvider(
new RelationalAnnotationProviderDependencies()));
var conventionSetDependencies = TestHelpers.CreateContextServices()
.GetRequiredService<ProviderConventionSetBuilderDependencies>();
var relationalConventionSetDependencies = TestHelpers.CreateContextServices()
.GetRequiredService<RelationalConventionSetBuilderDependencies>();
conventions.ModelFinalizingConventions.Add(new TypeMappingConvention(conventionSetDependencies));
conventions.ModelFinalizedConventions.Add(new RelationalModelConvention(conventionSetDependencies, relationalConventionSetDependencies));
return conventions;
}

protected virtual MigrationsModelDiffer CreateModelDiffer(DbContextOptions options)
{
var ctx = TestHelpers.CreateContext(options);
var context = TestHelpers.CreateContext(options);
return new MigrationsModelDiffer(
new TestRelationalTypeMappingSource(
TestServiceFactory.Instance.Create<TypeMappingSourceDependencies>(),
TestServiceFactory.Instance.Create<RelationalTypeMappingSourceDependencies>()),
new MigrationsAnnotationProvider(
new MigrationsAnnotationProviderDependencies()),
ctx.GetService<IChangeDetector>(),
ctx.GetService<IUpdateAdapterFactory>(),
ctx.GetService<CommandBatchPreparerDependencies>());
context.GetService<IChangeDetector>(),
context.GetService<IUpdateAdapterFactory>(),
context.GetService<CommandBatchPreparerDependencies>());
}
}
}
Loading

0 comments on commit 59e1277

Please sign in to comment.