Skip to content

Commit

Permalink
Fix for 15759. Update an error message for FK properties mismatch (#2…
Browse files Browse the repository at this point in the history
…0616)

Fix for 15759. Update the error message to include the entity types or navigations involved.
  • Loading branch information
lajones committed Apr 14, 2020
1 parent 2ffa15d commit 5f4fab4
Show file tree
Hide file tree
Showing 7 changed files with 97 additions and 13 deletions.
16 changes: 13 additions & 3 deletions src/EFCore/Diagnostics/CoreLoggerExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1081,10 +1081,14 @@ private static string RedundantForeignKeyWarning(EventDefinitionBase definition,
/// Logs for the <see cref="CoreEventId.IncompatibleMatchingForeignKeyProperties" /> event.
/// </summary>
/// <param name="diagnostics"> The diagnostics logger to use. </param>
/// <param name="dependentToPrincipalNavigationSpecification"> The name of the navigation property or entity type on the dependent end of the relationship. </param>
/// <param name="principalToDependentNavigationSpecification"> The name of the navigation property or entity type on the principal end of the relationship. </param>
/// <param name="foreignKeyProperties"> The properties that make up the foreign key. </param>
/// <param name="principalKeyProperties"> The corresponding keys on the principal side. </param>
public static void IncompatibleMatchingForeignKeyProperties(
[NotNull] this IDiagnosticsLogger<DbLoggerCategory.Model> diagnostics,
[NotNull] string dependentToPrincipalNavigationSpecification,
[NotNull] string principalToDependentNavigationSpecification,
[NotNull] IReadOnlyList<IPropertyBase> foreignKeyProperties,
[NotNull] IReadOnlyList<IPropertyBase> principalKeyProperties)
{
Expand All @@ -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);

Expand All @@ -1112,9 +1120,11 @@ public static void IncompatibleMatchingForeignKeyProperties(

private static string IncompatibleMatchingForeignKeyProperties(EventDefinitionBase definition, EventData payload)
{
var d = (EventDefinition<string, string>)definition;
var p = (TwoPropertyBaseCollectionsEventData)payload;
var d = (EventDefinition<string, string, string, string>)definition;
var p = (ForeignKeyCandidateEventData)payload;
return d.GenerateMessage(
p.DependentToPrincipalNavigationSpecification,
p.PrincipalToDependentNavigationSpecification,
p.FirstPropertyCollection.Format(includeTypes: true),
p.SecondPropertyCollection.Format(includeTypes: true));
}
Expand Down
50 changes: 50 additions & 0 deletions src/EFCore/Diagnostics/ForeignKeyCandidateEventData.cs
Original file line number Diff line number Diff line change
@@ -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
{
/// <summary>
/// A <see cref="DiagnosticSource" /> event payload class for
/// incompatible foreign key properties.
/// </summary>
public class ForeignKeyCandidateEventData : TwoPropertyBaseCollectionsEventData
{
/// <summary>
/// Constructs the event payload.
/// </summary>
/// <param name="eventDefinition"> The event definition. </param>
/// <param name="messageGenerator"> A delegate that generates a log message for this event. </param>
/// <param name="dependentToPrincipalNavigationSpecification"> The name of the navigation property or entity type on the dependent end of the relationship. </param>
/// <param name="principalToDependentNavigationSpecification"> The name of the navigation property or entity type on the principal end of the relationship. </param>
/// <param name="firstPropertyCollection"> The first property collection. </param>
/// <param name="secondPropertyCollection"> The second property collection. </param>
public ForeignKeyCandidateEventData(
[NotNull] EventDefinitionBase eventDefinition,
[NotNull] Func<EventDefinitionBase, EventData, string> messageGenerator,
[NotNull] string dependentToPrincipalNavigationSpecification,
[NotNull] string principalToDependentNavigationSpecification,
[NotNull] IReadOnlyList<IPropertyBase> firstPropertyCollection,
[NotNull] IReadOnlyList<IPropertyBase> secondPropertyCollection)
: base(eventDefinition, messageGenerator, firstPropertyCollection, secondPropertyCollection)
{
DependentToPrincipalNavigationSpecification = dependentToPrincipalNavigationSpecification;
PrincipalToDependentNavigationSpecification = principalToDependentNavigationSpecification;
}

/// <summary>
/// The name of the navigation property or entity type on the dependent end of the relationship.
/// </summary>
public virtual string DependentToPrincipalNavigationSpecification { get; }

/// <summary>
/// The name of the navigation property or entity type on the principal end of the relationship.
/// </summary>
public virtual string PrincipalToDependentNavigationSpecification { get; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions src/EFCore/Properties/CoreStrings.Designer.cs

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

4 changes: 2 additions & 2 deletions src/EFCore/Properties/CoreStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -992,8 +992,8 @@
<comment>Debug CoreEventId.RedundantIndexRemoved string string string</comment>
</data>
<data name="LogIncompatibleMatchingForeignKeyProperties" xml:space="preserve">
<value>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.</value>
<comment>Debug CoreEventId.IncompatibleMatchingForeignKeyProperties string string</comment>
<value>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.</value>
<comment>Debug CoreEventId.IncompatibleMatchingForeignKeyProperties string string string string</comment>
</data>
<data name="LogRequiredAttributeInverted" xml:space="preserve">
<value>The navigation property '{navigation}' has a RequiredAttribute causing the entity type '{entityType}' to be configured as the dependent side in the corresponding relationship.</value>
Expand Down
1 change: 1 addition & 0 deletions test/EFCore.Tests/Infrastructure/CoreEventIdTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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() },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<TestLoggingDefinitions>()).GenerateMessage(
"{'PrincipalEntityPeeKay' : string}", "{'PeeKay' : int}"), logEntry.Message);
CoreResources.LogIncompatibleMatchingForeignKeyProperties(
new TestLogger<TestLoggingDefinitions>()).GenerateMessage(
nameof(DependentEntity) + "." + nameof(DependentEntity.SomeNav),
nameof(PrincipalEntity),
"{'PrincipalEntityPeeKay' : string}",
"{'PeeKay' : int}"),
logEntry.Message);

ValidateModel();
}
Expand Down

0 comments on commit 5f4fab4

Please sign in to comment.