From 5f4fab48562161b140d891cb56f4c03873a17adc Mon Sep 17 00:00:00 2001 From: lajones Date: Tue, 14 Apr 2020 15:02:12 -0700 Subject: [PATCH] Fix for 15759. Update an error message for FK properties mismatch (#20616) Fix for 15759. Update the error message to include the entity types or navigations involved. --- .../Diagnostics/CoreLoggerExtensions.cs | 16 ++++-- .../ForeignKeyCandidateEventData.cs | 50 +++++++++++++++++++ .../ForeignKeyPropertyDiscoveryConvention.cs | 20 +++++++- src/EFCore/Properties/CoreStrings.Designer.cs | 10 ++-- src/EFCore/Properties/CoreStrings.resx | 4 +- .../Infrastructure/CoreEventIdTest.cs | 1 + ...reignKeyPropertyDiscoveryConventionTest.cs | 9 +++- 7 files changed, 97 insertions(+), 13 deletions(-) create mode 100644 src/EFCore/Diagnostics/ForeignKeyCandidateEventData.cs diff --git a/src/EFCore/Diagnostics/CoreLoggerExtensions.cs b/src/EFCore/Diagnostics/CoreLoggerExtensions.cs index a64f8d77d93..c9046172cdf 100644 --- a/src/EFCore/Diagnostics/CoreLoggerExtensions.cs +++ b/src/EFCore/Diagnostics/CoreLoggerExtensions.cs @@ -1081,10 +1081,14 @@ private static string RedundantForeignKeyWarning(EventDefinitionBase definition, /// Logs for the event. /// /// The diagnostics logger to use. + /// The name of the navigation property or entity type on the dependent end of the relationship. + /// The name of the navigation property or entity type on the principal end of the relationship. /// The properties that make up the foreign key. /// The corresponding keys on the principal side. public static void IncompatibleMatchingForeignKeyProperties( [NotNull] this IDiagnosticsLogger diagnostics, + [NotNull] string dependentToPrincipalNavigationSpecification, + [NotNull] string principalToDependentNavigationSpecification, [NotNull] IReadOnlyList foreignKeyProperties, [NotNull] IReadOnlyList principalKeyProperties) { @@ -1094,15 +1098,19 @@ public static void IncompatibleMatchingForeignKeyProperties( { definition.Log( diagnostics, + dependentToPrincipalNavigationSpecification, + principalToDependentNavigationSpecification, foreignKeyProperties.Format(includeTypes: true), principalKeyProperties.Format(includeTypes: true)); } if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled)) { - var eventData = new TwoPropertyBaseCollectionsEventData( + var eventData = new ForeignKeyCandidateEventData( definition, IncompatibleMatchingForeignKeyProperties, + dependentToPrincipalNavigationSpecification, + principalToDependentNavigationSpecification, foreignKeyProperties, principalKeyProperties); @@ -1112,9 +1120,11 @@ public static void IncompatibleMatchingForeignKeyProperties( private static string IncompatibleMatchingForeignKeyProperties(EventDefinitionBase definition, EventData payload) { - var d = (EventDefinition)definition; - var p = (TwoPropertyBaseCollectionsEventData)payload; + var d = (EventDefinition)definition; + var p = (ForeignKeyCandidateEventData)payload; return d.GenerateMessage( + p.DependentToPrincipalNavigationSpecification, + p.PrincipalToDependentNavigationSpecification, p.FirstPropertyCollection.Format(includeTypes: true), p.SecondPropertyCollection.Format(includeTypes: true)); } diff --git a/src/EFCore/Diagnostics/ForeignKeyCandidateEventData.cs b/src/EFCore/Diagnostics/ForeignKeyCandidateEventData.cs new file mode 100644 index 00000000000..29c70cb6950 --- /dev/null +++ b/src/EFCore/Diagnostics/ForeignKeyCandidateEventData.cs @@ -0,0 +1,50 @@ +// 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 System.Collections.Generic; +using System.Diagnostics; +using JetBrains.Annotations; +using Microsoft.EntityFrameworkCore.Metadata; + +namespace Microsoft.EntityFrameworkCore.Diagnostics +{ + /// + /// A event payload class for + /// incompatible foreign key properties. + /// + public class ForeignKeyCandidateEventData : TwoPropertyBaseCollectionsEventData + { + /// + /// Constructs the event payload. + /// + /// The event definition. + /// A delegate that generates a log message for this event. + /// The name of the navigation property or entity type on the dependent end of the relationship. + /// The name of the navigation property or entity type on the principal end of the relationship. + /// The first property collection. + /// The second property collection. + public ForeignKeyCandidateEventData( + [NotNull] EventDefinitionBase eventDefinition, + [NotNull] Func messageGenerator, + [NotNull] string dependentToPrincipalNavigationSpecification, + [NotNull] string principalToDependentNavigationSpecification, + [NotNull] IReadOnlyList firstPropertyCollection, + [NotNull] IReadOnlyList secondPropertyCollection) + : base(eventDefinition, messageGenerator, firstPropertyCollection, secondPropertyCollection) + { + DependentToPrincipalNavigationSpecification = dependentToPrincipalNavigationSpecification; + PrincipalToDependentNavigationSpecification = principalToDependentNavigationSpecification; + } + + /// + /// The name of the navigation property or entity type on the dependent end of the relationship. + /// + public virtual string DependentToPrincipalNavigationSpecification { get; } + + /// + /// The name of the navigation property or entity type on the principal end of the relationship. + /// + public virtual string PrincipalToDependentNavigationSpecification { get; } + } +} diff --git a/src/EFCore/Metadata/Conventions/ForeignKeyPropertyDiscoveryConvention.cs b/src/EFCore/Metadata/Conventions/ForeignKeyPropertyDiscoveryConvention.cs index b16ed7004de..43280026618 100644 --- a/src/EFCore/Metadata/Conventions/ForeignKeyPropertyDiscoveryConvention.cs +++ b/src/EFCore/Metadata/Conventions/ForeignKeyPropertyDiscoveryConvention.cs @@ -418,7 +418,25 @@ private bool TryFindMatchingProperties( p => !p.IsShadowProperty() || p.GetConfigurationSource().Overrides(ConfigurationSource.DataAnnotation))) { - Dependencies.Logger.IncompatibleMatchingForeignKeyProperties(foreignKeyProperties, propertiesToReference); + var dependentNavigationSpec = onDependent + ? foreignKey.DependentToPrincipal?.Name + : foreignKey.PrincipalToDependent?.Name; + dependentNavigationSpec = dependentEntityType.DisplayName() + + (string.IsNullOrEmpty(dependentNavigationSpec) + ? string.Empty + : "." + dependentNavigationSpec); + + var principalNavigationSpec = onDependent + ? foreignKey.PrincipalToDependent?.Name + : foreignKey.DependentToPrincipal?.Name; + principalNavigationSpec = principalEntityType.DisplayName() + + (string.IsNullOrEmpty(principalNavigationSpec) + ? string.Empty + : "." + principalNavigationSpec); + + Dependencies.Logger.IncompatibleMatchingForeignKeyProperties( + dependentNavigationSpec, principalNavigationSpec, + foreignKeyProperties, propertiesToReference); } // Stop searching if match found, but is incompatible diff --git a/src/EFCore/Properties/CoreStrings.Designer.cs b/src/EFCore/Properties/CoreStrings.Designer.cs index 8717cbd083a..cf4bafc35de 100644 --- a/src/EFCore/Properties/CoreStrings.Designer.cs +++ b/src/EFCore/Properties/CoreStrings.Designer.cs @@ -3671,27 +3671,27 @@ public static EventDefinition LogRedundantIndexRemoved([ } /// - /// The foreign key properties haven't been configured by convention because the best match {foreignKey} are incompatible with the current principal key {principalKey}. This message can be disregarded if explicit configuration has been specified. + /// For the relationship between dependent '{dependentToPrincipalNavigationSpecification}' and principal '{principalToDependentNavigationSpecification}', the foreign key properties haven't been configured by convention because the best match {foreignKey} are incompatible with the current principal key {principalKey}. This message can be disregarded if explicit configuration has been specified. /// - public static EventDefinition LogIncompatibleMatchingForeignKeyProperties([NotNull] IDiagnosticsLogger logger) + public static EventDefinition LogIncompatibleMatchingForeignKeyProperties([NotNull] IDiagnosticsLogger logger) { var definition = ((LoggingDefinitions)logger.Definitions).LogIncompatibleMatchingForeignKeyProperties; if (definition == null) { definition = LazyInitializer.EnsureInitialized( ref ((LoggingDefinitions)logger.Definitions).LogIncompatibleMatchingForeignKeyProperties, - () => new EventDefinition( + () => new EventDefinition( logger.Options, CoreEventId.IncompatibleMatchingForeignKeyProperties, LogLevel.Debug, "CoreEventId.IncompatibleMatchingForeignKeyProperties", - level => LoggerMessage.Define( + level => LoggerMessage.Define( level, CoreEventId.IncompatibleMatchingForeignKeyProperties, _resourceManager.GetString("LogIncompatibleMatchingForeignKeyProperties")))); } - return (EventDefinition)definition; + return (EventDefinition)definition; } /// diff --git a/src/EFCore/Properties/CoreStrings.resx b/src/EFCore/Properties/CoreStrings.resx index 8e3919f7488..76248cb542f 100644 --- a/src/EFCore/Properties/CoreStrings.resx +++ b/src/EFCore/Properties/CoreStrings.resx @@ -992,8 +992,8 @@ Debug CoreEventId.RedundantIndexRemoved string string string - The foreign key properties haven't been configured by convention because the best match {foreignKey} are incompatible with the current principal key {principalKey}. This message can be disregarded if explicit configuration has been specified. - Debug CoreEventId.IncompatibleMatchingForeignKeyProperties string string + For the relationship between dependent '{dependentToPrincipalNavigationSpecification}' and principal '{principalToDependentNavigationSpecification}', the foreign key properties haven't been configured by convention because the best match {foreignKey} are incompatible with the current principal key {principalKey}. This message can be disregarded if explicit configuration has been specified. + Debug CoreEventId.IncompatibleMatchingForeignKeyProperties string string string string The navigation property '{navigation}' has a RequiredAttribute causing the entity type '{entityType}' to be configured as the dependent side in the corresponding relationship. diff --git a/test/EFCore.Tests/Infrastructure/CoreEventIdTest.cs b/test/EFCore.Tests/Infrastructure/CoreEventIdTest.cs index 2992a1bbbe8..23dd6850ce5 100644 --- a/test/EFCore.Tests/Infrastructure/CoreEventIdTest.cs +++ b/test/EFCore.Tests/Infrastructure/CoreEventIdTest.cs @@ -45,6 +45,7 @@ public void Every_eventId_has_a_logger_method_and_logs_when_level_enabled() { typeof(ExpressionPrinter), () => new ExpressionPrinter() }, { typeof(Expression), () => Expression.Constant("A") }, { typeof(IEntityType), () => entityType }, + { typeof(IConventionEntityType), () => entityType }, { typeof(IKey), () => new Key(new[] { property }, ConfigurationSource.Convention) }, { typeof(IPropertyBase), () => property }, { typeof(IServiceProvider), () => new FakeServiceProvider() }, diff --git a/test/EFCore.Tests/Metadata/Conventions/ForeignKeyPropertyDiscoveryConventionTest.cs b/test/EFCore.Tests/Metadata/Conventions/ForeignKeyPropertyDiscoveryConventionTest.cs index 4c01f3411a3..9dbf2005808 100644 --- a/test/EFCore.Tests/Metadata/Conventions/ForeignKeyPropertyDiscoveryConventionTest.cs +++ b/test/EFCore.Tests/Metadata/Conventions/ForeignKeyPropertyDiscoveryConventionTest.cs @@ -506,8 +506,13 @@ public void Does_not_match_principal_type_plus_PK_name_property_of_different_typ var logEntry = ListLoggerFactory.Log.Single(); Assert.Equal(LogLevel.Debug, logEntry.Level); Assert.Equal( - CoreResources.LogIncompatibleMatchingForeignKeyProperties(new TestLogger()).GenerateMessage( - "{'PrincipalEntityPeeKay' : string}", "{'PeeKay' : int}"), logEntry.Message); + CoreResources.LogIncompatibleMatchingForeignKeyProperties( + new TestLogger()).GenerateMessage( + nameof(DependentEntity) + "." + nameof(DependentEntity.SomeNav), + nameof(PrincipalEntity), + "{'PrincipalEntityPeeKay' : string}", + "{'PeeKay' : int}"), + logEntry.Message); ValidateModel(); }