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

Add a warning for unmapped foreign keys #22175

Merged
1 commit merged into from
Aug 24, 2020
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
14 changes: 14 additions & 0 deletions src/EFCore.Relational/Diagnostics/RelationalEventId.cs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ private enum Id
AllIndexPropertiesNotToMappedToAnyTable,
IndexPropertiesBothMappedAndNotMappedToTable,
IndexPropertiesMappedToNonOverlappingTables,
ForeignKeyPropertiesMappedToUnrelatedTables,

// Update events
BatchReadyForExecution = CoreEventId.RelationalBaseId + 700,
Expand Down Expand Up @@ -709,6 +710,19 @@ private static EventId MakeValidationId(Id id)
public static readonly EventId IndexPropertiesMappedToNonOverlappingTables =
MakeValidationId(Id.IndexPropertiesMappedToNonOverlappingTables);

/// <summary>
/// <para>
/// A foreign key specifies properties which don't map to the related tables.
/// </para>
/// <para>
/// This event is in the <see cref="DbLoggerCategory.Model.Validation" /> category.
/// </para>
/// <para>
/// This event uses the <see cref="ForeignKeyEventData" /> payload when used with a <see cref="DiagnosticSource" />.
/// </para>
/// </summary>
public static readonly EventId ForeignKeyPropertiesMappedToUnrelatedTables = MakeValidationId(Id.ForeignKeyPropertiesMappedToUnrelatedTables);

private static readonly string _updatePrefix = DbLoggerCategory.Update.Name + ".";

private static EventId MakeUpdateId(Id id)
Expand Down
56 changes: 56 additions & 0 deletions src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4650,6 +4650,62 @@ private static string NamedIndexPropertiesMappedToNonOverlappingTables(EventDefi
p.TablesMappedToProperty2.FormatTables()));
}

/// <summary>
/// Logs the <see cref="RelationalEventId.IndexPropertiesMappedToNonOverlappingTables" /> event.
/// </summary>
/// <param name="diagnostics"> The diagnostics logger to use. </param>
/// <param name="foreignKey"> The foreign key. </param>
public static void ForeignKeyPropertiesMappedToUnrelatedTables(
[NotNull] this IDiagnosticsLogger<DbLoggerCategory.Model.Validation> diagnostics,
[NotNull] IForeignKey foreignKey)
{
var definition = RelationalResources.LogForeignKeyPropertiesMappedToUnrelatedTables(diagnostics);

if (diagnostics.ShouldLog(definition))
{
definition.Log(diagnostics,
l => l.Log(
definition.Level,
definition.EventId,
definition.MessageFormat,
foreignKey.Properties.Format(),
foreignKey.DeclaringEntityType.DisplayName(),
foreignKey.PrincipalEntityType.DisplayName(),
foreignKey.Properties.Format(),
foreignKey.DeclaringEntityType.GetSchemaQualifiedTableName(),
foreignKey.PrincipalKey.Properties.Format(),
foreignKey.PrincipalEntityType.GetSchemaQualifiedTableName()));
}

if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
{
var eventData = new ForeignKeyEventData(
definition,
ForeignKeyPropertiesMappedToUnrelatedTables,
foreignKey);

diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled);
}
}

private static string ForeignKeyPropertiesMappedToUnrelatedTables(EventDefinitionBase definition, EventData payload)
{
var d = (FallbackEventDefinition)definition;
var p = (ForeignKeyEventData)payload;
return d.GenerateMessage(
l => l.Log(
d.Level,
d.EventId,
d.MessageFormat,
p.ForeignKey.Properties.Format(),
p.ForeignKey.DeclaringEntityType.DisplayName(),
p.ForeignKey.PrincipalEntityType.DisplayName(),
p.ForeignKey.Properties.Format(),
p.ForeignKey.DeclaringEntityType.GetSchemaQualifiedTableName(),
p.ForeignKey.PrincipalKey.Properties.Format(),
p.ForeignKey.PrincipalEntityType.GetSchemaQualifiedTableName()));
}

/// <summary>
/// Logs for the <see cref="RelationalEventId.BatchExecutorFailedToRollbackToSavepoint" /> event.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,15 @@ public abstract class RelationalLoggingDefinitions : LoggingDefinitions
[EntityFrameworkInternal]
public EventDefinitionBase LogUnnamedIndexPropertiesMappedToNonOverlappingTables;

/// <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>
[EntityFrameworkInternal]
public EventDefinitionBase LogForeignKeyPropertiesMappedToUnrelatedTables;

/// <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
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) .NET Foundation. All rights reserved.
// 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.Collections.Generic;
Expand All @@ -23,13 +23,12 @@ public static class RelationalForeignKeyExtensions
/// <param name="foreignKey"> The foreign key. </param>
/// <returns> The foreign key constraint name. </returns>
public static string GetConstraintName([NotNull] this IForeignKey foreignKey)
=> foreignKey.GetConstraintName(
StoreObjectIdentifier.Create(foreignKey.DeclaringEntityType, StoreObjectType.Table).Value,
StoreObjectIdentifier.Create(
foreignKey.PrincipalKey.IsPrimaryKey()
? foreignKey.PrincipalEntityType
: foreignKey.PrincipalKey.DeclaringEntityType,
StoreObjectType.Table).Value);
{
var annotation = foreignKey.FindAnnotation(RelationalAnnotationNames.Name);
return annotation != null
? (string)annotation.Value
: foreignKey.GetDefaultName();
}

/// <summary>
/// Returns the foreign key constraint name.
Expand Down
28 changes: 23 additions & 5 deletions src/EFCore.Relational/Extensions/RelationalPropertyExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,30 @@ 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)
if (storeObject.StoreObjectType != StoreObjectType.Function
&& storeObject.StoreObjectType != StoreObjectType.SqlQuery)
{
return null;
if (property.IsPrimaryKey())
{
var tableFound = false;
foreach (var containingType in property.DeclaringEntityType.GetDerivedTypesInclusive())
{
if (StoreObjectIdentifier.Create(containingType, storeObject.StoreObjectType) == storeObject)
{
tableFound = true;
break;
}
}

if (!tableFound)
{
return null;
}
}
else if (StoreObjectIdentifier.Create(property.DeclaringEntityType, storeObject.StoreObjectType) != storeObject)
{
return null;
}
}

var columnAnnotation = property.FindAnnotation(RelationalAnnotationNames.ColumnName);
Expand Down
19 changes: 13 additions & 6 deletions src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -924,6 +924,17 @@ protected virtual void ValidateSharedForeignKeysCompatibility(
var foreignKeyName = foreignKey.GetConstraintName(storeObject, principalTable.Value);
if (foreignKeyName == null)
{
var derivedTables = foreignKey.DeclaringEntityType.GetDerivedTypes()
.Select(t => StoreObjectIdentifier.Create(t, StoreObjectType.Table))
.Where(t => t != null);
if (foreignKey.GetConstraintName() != null
&& derivedTables.All(t => foreignKey.GetConstraintName(
t.Value,
principalTable.Value) == null))
{
logger.ForeignKeyPropertiesMappedToUnrelatedTables(foreignKey);
}

continue;
}

Expand Down Expand Up @@ -1254,8 +1265,7 @@ protected virtual void ValidateIndexProperties(

if (firstPropertyTables != null)
{
// Property is not mapped but we already found
// a property that is mapped.
// Property is not mapped but we already found a property that is mapped.
break;
}

Expand All @@ -1264,21 +1274,18 @@ protected virtual void ValidateIndexProperties(

if (firstPropertyTables == null)
{
// store off which tables the first member maps to
firstPropertyTables =
new Tuple<string, List<(string Table, string Schema)>>(property.Name, tablesMappedToProperty);
}
else
{
// store off which tables the last member we encountered maps to
lastPropertyTables =
new Tuple<string, List<(string Table, string Schema)>>(property.Name, tablesMappedToProperty);
}

if (propertyNotMappedToAnyTable != null)
{
// Property is mapped but we already found
// a property that is not mapped.
// Property is mapped but we already found a property that is not mapped.
overlappingTables = null;
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,18 @@ public virtual void Validate(IDbContextOptions options)
{
}

/// <summary>
/// Adds default <see cref="WarningBehavior"/> for relational events.
/// </summary>
/// <param name="coreOptionsExtension"> The core options extension. </param>
/// <returns> The new core options extension. </returns>
public static CoreOptionsExtension WithDefaultWarningConfiguration([NotNull] CoreOptionsExtension coreOptionsExtension)
=> coreOptionsExtension.WithWarningsConfiguration(coreOptionsExtension.WarningsConfiguration
.TryWithExplicit(RelationalEventId.AmbientTransactionWarning, WarningBehavior.Throw)
.TryWithExplicit(RelationalEventId.IndexPropertiesBothMappedAndNotMappedToTable, WarningBehavior.Throw)
.TryWithExplicit(RelationalEventId.IndexPropertiesMappedToNonOverlappingTables, WarningBehavior.Throw)
.TryWithExplicit(RelationalEventId.ForeignKeyPropertiesMappedToUnrelatedTables, WarningBehavior.Throw));

/// <summary>
/// Information/metadata for a <see cref="RelationalOptionsExtension" />.
/// </summary>
Expand All @@ -396,7 +408,7 @@ protected abstract class RelationalExtensionInfo : DbContextOptionsExtensionInfo
private string _logFragment;

/// <summary>
/// Creates a new <see cref="RelationalOptionsExtension.RelationalExtensionInfo" /> instance containing
/// Creates a new <see cref="RelationalExtensionInfo" /> instance containing
/// info/metadata for the given extension.
/// </summary>
/// <param name="extension"> The extension. </param>
Expand Down
14 changes: 11 additions & 3 deletions src/EFCore.Relational/Metadata/Internal/RelationalModel.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) .NET Foundation. All rights reserved.
// 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;
Expand Down Expand Up @@ -830,9 +830,17 @@ private static void PopulateConstraints(Table table)
break;
}

if (entityTypeMapping.IncludesDerivedTypes
&& foreignKey.DeclaringEntityType != entityType
&& foreignKey.Properties.SequenceEqual(entityType.FindPrimaryKey().Properties))
{
// The identifying FK constraint is needed to be created only on the table that corresponds
// to the declaring entity type
break;
}

constraint = new ForeignKeyConstraint(
name, table, principalTable, columns, principalColumns,
ToReferentialAction(foreignKey.DeleteBehavior));
name, table, principalTable, columns, principalColumns, ToReferentialAction(foreignKey.DeleteBehavior));
constraint.MappedForeignKeys.Add(foreignKey);

if (foreignKeyConstraints == null)
Expand Down
21 changes: 21 additions & 0 deletions src/EFCore.Relational/Properties/RelationalStrings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions src/EFCore.Relational/Properties/RelationalStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,10 @@
<value>Enlisted in an explicit transaction with isolation level '{isolationLevel}'.</value>
<comment>Debug RelationalEventId.ExplicitTransactionEnlisted string</comment>
</data>
<data name="LogForeignKeyPropertiesMappedToUnrelatedTables" xml:space="preserve">
<value>The foreign key {foreignKeyProperties} on the entity type '{entityType}' targeting '{principalEntityType}' cannot be represented in the database. Either the properties {foreignKeyProperties} aren't mapped to table '{table}' or the principal properties {principalProperties} aren't mapped to table '{principalTable}'. All foreign key properties must map to the table that the dependent type is mapped to and all principal properties must map to a single table that the principal type is mapped to.</value>
<comment>Error RelationalEventId.ForeignKeyPropertiesMappedToUnrelatedTables string string string string string string string</comment>
</data>
<data name="LogGeneratingDown" xml:space="preserve">
<value>Generating down script for migration '{migration}'.</value>
<comment>Debug RelationalEventId.MigrationGeneratingDownScript string</comment>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) .NET Foundation. All rights reserved.
// 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;
Expand Down Expand Up @@ -169,10 +169,8 @@ var coreOptionsExtension
= optionsBuilder.Options.FindExtension<CoreOptionsExtension>()
?? new CoreOptionsExtension();

coreOptionsExtension = coreOptionsExtension.WithWarningsConfiguration(
coreOptionsExtension.WarningsConfiguration.TryWithExplicit(
RelationalEventId.AmbientTransactionWarning, WarningBehavior.Throw)).WithWarningsConfiguration(
coreOptionsExtension.WarningsConfiguration.TryWithExplicit(
coreOptionsExtension = RelationalOptionsExtension.WithDefaultWarningConfiguration(coreOptionsExtension)
.WithWarningsConfiguration(coreOptionsExtension.WarningsConfiguration.TryWithExplicit(
SqlServerEventId.ConflictingValueGenerationStrategiesWarning, WarningBehavior.Throw));

((IDbContextOptionsBuilderInfrastructure)optionsBuilder).AddOrUpdateExtension(coreOptionsExtension);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,7 @@ var coreOptionsExtension
= optionsBuilder.Options.FindExtension<CoreOptionsExtension>()
?? new CoreOptionsExtension();

coreOptionsExtension = coreOptionsExtension.WithWarningsConfiguration(
coreOptionsExtension.WarningsConfiguration.TryWithExplicit(
RelationalEventId.AmbientTransactionWarning, WarningBehavior.Throw));
coreOptionsExtension = RelationalOptionsExtension.WithDefaultWarningConfiguration(coreOptionsExtension);

((IDbContextOptionsBuilderInfrastructure)optionsBuilder).AddOrUpdateExtension(coreOptionsExtension);
}
Expand Down
Loading