From b6b911f42e5060c267ca77cbbb182ac566903ec1 Mon Sep 17 00:00:00 2001 From: Smit Patel Date: Thu, 2 Sep 2021 13:50:16 -0700 Subject: [PATCH] SqlServer: Ignore Fks whose principal table data is not found Resolves #23359 --- .../Internal/SqlServerLoggingDefinitions.cs | 8 ++ .../Diagnostics/SqlServerEventId.cs | 10 ++- .../Internal/SqlServerLoggerExtensions.cs | 23 +++++- .../Properties/SqlServerStrings.Designer.cs | 78 +++++++++++++------ .../Properties/SqlServerStrings.resx | 12 ++- .../Internal/SqlServerDatabaseModelFactory.cs | 19 +++-- .../SqlServerDatabaseModelFactoryTest.cs | 2 +- 7 files changed, 115 insertions(+), 37 deletions(-) diff --git a/src/EFCore.SqlServer/Diagnostics/Internal/SqlServerLoggingDefinitions.cs b/src/EFCore.SqlServer/Diagnostics/Internal/SqlServerLoggingDefinitions.cs index 150aee42191..3dfcfbc4b2f 100644 --- a/src/EFCore.SqlServer/Diagnostics/Internal/SqlServerLoggingDefinitions.cs +++ b/src/EFCore.SqlServer/Diagnostics/Internal/SqlServerLoggingDefinitions.cs @@ -165,6 +165,14 @@ public class SqlServerLoggingDefinitions : RelationalLoggingDefinitions /// public EventDefinitionBase? LogDuplicateForeignKeyConstraintIgnored; + /// + /// 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. + /// + public EventDefinitionBase? LogPrincipalTableInformationNotFound; + /// /// 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 diff --git a/src/EFCore.SqlServer/Diagnostics/SqlServerEventId.cs b/src/EFCore.SqlServer/Diagnostics/SqlServerEventId.cs index 833d790f3df..82f65737858 100644 --- a/src/EFCore.SqlServer/Diagnostics/SqlServerEventId.cs +++ b/src/EFCore.SqlServer/Diagnostics/SqlServerEventId.cs @@ -64,7 +64,8 @@ private enum Id ForeignKeyPrincipalColumnMissingWarning, ReflexiveConstraintIgnored, DuplicateForeignKeyConstraintIgnored, - ColumnWithoutTypeWarning + ColumnWithoutTypeWarning, + ForeignKeyReferencesUnknownPrincipalTableWarning } private static readonly string _validationPrefix = DbLoggerCategory.Model.Validation.Name + "."; @@ -184,6 +185,13 @@ private static EventId MakeScaffoldingId(Id id) public static readonly EventId ForeignKeyReferencesMissingPrincipalTableWarning = MakeScaffoldingId(Id.ForeignKeyReferencesMissingPrincipalTableWarning); + /// + /// A foreign key references a unknown table at the principal end. + /// This event is in the category. + /// + public static readonly EventId ForeignKeyReferencesUnknownPrincipalTableWarning = + MakeScaffoldingId(Id.ForeignKeyReferencesUnknownPrincipalTableWarning); + /// /// A table was found. /// This event is in the category. diff --git a/src/EFCore.SqlServer/Extensions/Internal/SqlServerLoggerExtensions.cs b/src/EFCore.SqlServer/Extensions/Internal/SqlServerLoggerExtensions.cs index aab255bb3ab..e6f0965a6b0 100644 --- a/src/EFCore.SqlServer/Extensions/Internal/SqlServerLoggerExtensions.cs +++ b/src/EFCore.SqlServer/Extensions/Internal/SqlServerLoggerExtensions.cs @@ -348,6 +348,27 @@ public static void IndexFound( // No DiagnosticsSource events because these are purely design-time messages } + /// + /// 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. + /// + public static void ForeignKeyReferencesUnknownPrincipalTableWarning( + this IDiagnosticsLogger diagnostics, + string? foreignKeyName, + string? tableName) + { + var definition = SqlServerResources.LogPrincipalTableInformationNotFound(diagnostics); + + if (diagnostics.ShouldLog(definition)) + { + definition.Log(diagnostics, foreignKeyName, tableName); + } + + // No DiagnosticsSource events because these are purely design-time messages + } + /// /// 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 @@ -544,7 +565,7 @@ public static void DuplicateForeignKeyConstraintIgnored( string tableName, string duplicateForeignKeyName) { - var definition = SqlServerResources.DuplicateForeignKeyConstraintIgnored(diagnostics); + var definition = SqlServerResources.LogDuplicateForeignKeyConstraintIgnored(diagnostics); if (diagnostics.ShouldLog(definition)) { diff --git a/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs b/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs index 4ccab03a1c4..23fd122e7e9 100644 --- a/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs +++ b/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs @@ -1,6 +1,9 @@ // +using System; +using System.Reflection; using System.Resources; +using System.Threading; using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Internal; using Microsoft.Extensions.Logging; @@ -485,6 +488,31 @@ public static EventDefinition LogDefaultDecimalTypeColumn(IDiagn return (EventDefinition)definition; } + /// + /// Skipping foreign key '{foreignKeyName}' on table '{tableName}' since it is a duplicate of '{duplicateForeignKeyName}'. + /// + public static EventDefinition LogDuplicateForeignKeyConstraintIgnored(IDiagnosticsLogger logger) + { + var definition = ((Diagnostics.Internal.SqlServerLoggingDefinitions)logger.Definitions).LogDuplicateForeignKeyConstraintIgnored; + if (definition == null) + { + definition = NonCapturingLazyInitializer.EnsureInitialized( + ref ((Diagnostics.Internal.SqlServerLoggingDefinitions)logger.Definitions).LogDuplicateForeignKeyConstraintIgnored, + logger, + static logger => new EventDefinition( + logger.Options, + SqlServerEventId.DuplicateForeignKeyConstraintIgnored, + LogLevel.Warning, + "SqlServerEventId.DuplicateForeignKeyConstraintIgnored", + level => LoggerMessage.Define( + level, + SqlServerEventId.DuplicateForeignKeyConstraintIgnored, + _resourceManager.GetString("LogDuplicateForeignKeyConstraintIgnored")!))); + } + + return (EventDefinition)definition; + } + /// /// Found column with table: {tableName}, column name: {columnName}, ordinal: {ordinal}, data type: {dataType}, maximum length: {maxLength}, precision: {precision}, scale: {scale}, nullable: {nullable}, identity: {identity}, default value: {defaultValue}, computed value: {computedValue}, computed value is stored: {stored}. /// @@ -779,6 +807,31 @@ public static EventDefinition LogPrincipalColumn return (EventDefinition)definition; } + /// + /// Skipping foreign key '{foreignKeyName}' on table '{tableName}' since principal table information is not available. This usually happens when the user doesn't have permission to read data about principal table. + /// + public static EventDefinition LogPrincipalTableInformationNotFound(IDiagnosticsLogger logger) + { + var definition = ((Diagnostics.Internal.SqlServerLoggingDefinitions)logger.Definitions).LogPrincipalTableInformationNotFound; + if (definition == null) + { + definition = NonCapturingLazyInitializer.EnsureInitialized( + ref ((Diagnostics.Internal.SqlServerLoggingDefinitions)logger.Definitions).LogPrincipalTableInformationNotFound, + logger, + static logger => new EventDefinition( + logger.Options, + SqlServerEventId.ForeignKeyReferencesUnknownPrincipalTableWarning, + LogLevel.Warning, + "SqlServerEventId.ForeignKeyReferencesUnknownPrincipalTableWarning", + level => LoggerMessage.Define( + level, + SqlServerEventId.ForeignKeyReferencesUnknownPrincipalTableWarning, + _resourceManager.GetString("LogPrincipalTableInformationNotFound")!))); + } + + return (EventDefinition)definition; + } + /// /// Skipping foreign key '{foreignKeyName}' on table '{tableName}' since principal table '{principalTableName}' was not found in the model. This usually happens when the principal table was not included in the selection set. /// @@ -829,31 +882,6 @@ public static EventDefinition LogReflexiveConstraintIgnored(IDia return (EventDefinition)definition; } - /// - /// Skipping foreign key '{foreignKeyName}' on table '{tableName}' since it is a duplicate of '{duplicateForeignKeyName}'. - /// - public static EventDefinition DuplicateForeignKeyConstraintIgnored(IDiagnosticsLogger logger) - { - var definition = ((Diagnostics.Internal.SqlServerLoggingDefinitions)logger.Definitions).LogDuplicateForeignKeyConstraintIgnored; - if (definition == null) - { - definition = NonCapturingLazyInitializer.EnsureInitialized( - ref ((Diagnostics.Internal.SqlServerLoggingDefinitions)logger.Definitions).LogDuplicateForeignKeyConstraintIgnored, - logger, - static logger => new EventDefinition( - logger.Options, - SqlServerEventId.DuplicateForeignKeyConstraintIgnored, - LogLevel.Warning, - "SqlServerEventId.DuplicateForeignKeyConstraintIgnored", - level => LoggerMessage.Define( - level, - SqlServerEventId.DuplicateForeignKeyConstraintIgnored, - _resourceManager.GetString("DuplicateForeignKeyConstraintIgnored")!))); - } - - return (EventDefinition)definition; - } - /// /// Savepoints are disabled because Multiple Active Result Sets (MARS) is enabled. If 'SaveChanges' fails, then the transaction cannot be automatically rolled back to a known clean state. Instead, the transaction should be rolled back by the application before retrying 'SaveChanges'. See https://go.microsoft.com/fwlink/?linkid=2149338 for more information. To identify the code which triggers this warning, call 'ConfigureWarnings(w => w.Throw(SqlServerEventId.SavepointsDisabledBecauseOfMARS))'. /// diff --git a/src/EFCore.SqlServer/Properties/SqlServerStrings.resx b/src/EFCore.SqlServer/Properties/SqlServerStrings.resx index e626bbedc50..5770c19ea28 100644 --- a/src/EFCore.SqlServer/Properties/SqlServerStrings.resx +++ b/src/EFCore.SqlServer/Properties/SqlServerStrings.resx @@ -200,6 +200,10 @@ No store type was specified for the decimal property '{property}' on entity type '{entityType}'. This will cause values to be silently truncated if they do not fit in the default precision and scale. Explicitly specify the SQL server column type that can accommodate all the values in 'OnModelCreating' using 'HasColumnType', specify precision and scale using 'HasPrecision', or configure a value converter using 'HasConversion'. Warning SqlServerEventId.DecimalTypeDefaultWarning string string + + Skipping foreign key '{foreignKeyName}' on table '{tableName}' since it is a duplicate of '{duplicateForeignKeyName}'. + Warning SqlServerEventId.DuplicateForeignKeyConstraintIgnored string string string + Found column with table: {tableName}, column name: {columnName}, ordinal: {ordinal}, data type: {dataType}, maximum length: {maxLength}, precision: {precision}, scale: {scale}, nullable: {nullable}, identity: {identity}, default value: {defaultValue}, computed value: {computedValue}, computed value is stored: {stored}. Debug SqlServerEventId.ColumnFound string string int string int int int bool bool string string bool? @@ -248,6 +252,10 @@ Skipping foreign key with identity '{id}' on table '{tableName}', since the principal column '{principalColumnName}' on the foreign key's principal table, '{principalTableName}', was not found in the model. Warning SqlServerEventId.ForeignKeyPrincipalColumnMissingWarning string string string string + + Skipping foreign key '{foreignKeyName}' on table '{tableName}' since principal table information is not available. This usually happens when the user doesn't have permission to read data about principal table. + Warning SqlServerEventId.ForeignKeyReferencesUnknownPrincipalTableWarning string? string? + Skipping foreign key '{foreignKeyName}' on table '{tableName}' since principal table '{principalTableName}' was not found in the model. This usually happens when the principal table was not included in the selection set. Warning SqlServerEventId.ForeignKeyReferencesMissingPrincipalTableWarning string? string? string? @@ -256,10 +264,6 @@ Skipping foreign key '{foreignKeyName}' on table '{tableName}' since all of its columns reference themselves. Debug SqlServerEventId.ReflexiveConstraintIgnored string string - - Skipping foreign key '{foreignKeyName}' on table '{tableName}' since it is a duplicate of '{duplicateForeignKeyName}'. - Warning SqlServerEventId.DuplicateForeignKeyConstraintIgnored string string string - Savepoints are disabled because Multiple Active Result Sets (MARS) is enabled. If 'SaveChanges' fails, then the transaction cannot be automatically rolled back to a known clean state. Instead, the transaction should be rolled back by the application before retrying 'SaveChanges'. See https://go.microsoft.com/fwlink/?linkid=2149338 for more information. To identify the code which triggers this warning, call 'ConfigureWarnings(w => w.Throw(SqlServerEventId.SavepointsDisabledBecauseOfMARS))'. Warning SqlServerEventId.SavepointsDisabledBecauseOfMARS diff --git a/src/EFCore.SqlServer/Scaffolding/Internal/SqlServerDatabaseModelFactory.cs b/src/EFCore.SqlServer/Scaffolding/Internal/SqlServerDatabaseModelFactory.cs index 20b1db2a489..dcac5e48bec 100644 --- a/src/EFCore.SqlServer/Scaffolding/Internal/SqlServerDatabaseModelFactory.cs +++ b/src/EFCore.SqlServer/Scaffolding/Internal/SqlServerDatabaseModelFactory.cs @@ -694,7 +694,7 @@ private void GetColumns( { commandText += @", [c].[generated_always_type]"; - } + } commandText += @"FROM ( @@ -1196,7 +1196,7 @@ FROM [sys].[foreign_keys] AS [f] .GroupBy( c => (Name: c.GetValueOrDefault("name"), PrincipalTableSchema: c.GetValueOrDefault("principal_table_schema"), - PrincipalTableName: c.GetFieldValue("principal_table_name"), + PrincipalTableName: c.GetValueOrDefault("principal_table_name"), OnDeleteAction: c.GetValueOrDefault("delete_referential_action_desc"))); foreach (var foreignKeyGroup in foreignKeyGroups) @@ -1206,9 +1206,18 @@ FROM [sys].[foreign_keys] AS [f] var principalTableName = foreignKeyGroup.Key.PrincipalTableName; var onDeleteAction = foreignKeyGroup.Key.OnDeleteAction; + if (principalTableName == null) + { + _logger.ForeignKeyReferencesUnknownPrincipalTableWarning( + fkName, + DisplayName(table.Schema, table.Name)); + + continue; + } + _logger.ForeignKeyFound( fkName!, - DisplayName(table.Schema, table.Name!), + DisplayName(table.Schema, table.Name), DisplayName(principalTableSchema, principalTableName), onDeleteAction!); @@ -1217,13 +1226,13 @@ FROM [sys].[foreign_keys] AS [f] && t.Name == principalTableName) ?? tables.FirstOrDefault( t => t.Schema?.Equals(principalTableSchema, StringComparison.OrdinalIgnoreCase) == true - && t.Name!.Equals(principalTableName, StringComparison.OrdinalIgnoreCase)); + && t.Name.Equals(principalTableName, StringComparison.OrdinalIgnoreCase)); if (principalTable == null) { _logger.ForeignKeyReferencesMissingPrincipalTableWarning( fkName, - DisplayName(table.Schema, table.Name!), + DisplayName(table.Schema, table.Name), DisplayName(principalTableSchema, principalTableName)); continue; diff --git a/test/EFCore.SqlServer.FunctionalTests/Scaffolding/SqlServerDatabaseModelFactoryTest.cs b/test/EFCore.SqlServer.FunctionalTests/Scaffolding/SqlServerDatabaseModelFactoryTest.cs index f58bf636340..1a835448a19 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Scaffolding/SqlServerDatabaseModelFactoryTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Scaffolding/SqlServerDatabaseModelFactoryTest.cs @@ -2422,7 +2422,7 @@ CONSTRAINT MYFK3 FOREIGN KEY (ForeignKeyId) REFERENCES OtherPrincipalTable(Id), Fixture.ListLoggerFactory.Log, t => t.Id == SqlServerEventId.DuplicateForeignKeyConstraintIgnored); Assert.Equal(LogLevel.Warning, level); Assert.Equal( - SqlServerResources.DuplicateForeignKeyConstraintIgnored(new TestLogger()) + SqlServerResources.LogDuplicateForeignKeyConstraintIgnored(new TestLogger()) .GenerateMessage("MYFK2", "dbo.DependentTable", "MYFK1"), message); var table = dbModel.Tables.Single(t => t.Name == "DependentTable");