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

Fix for 15759. Update an error message for FK properties mismatch #20616

Merged
merged 4 commits into from
Apr 14, 2020
Merged
Show file tree
Hide file tree
Changes from 2 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
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="principalEntityType"> The entity type on the principal end of the relationship. </param>
/// <param name="dependentEntityType"> The entity type on the dependent end of the relationship. </param>
/// <param name="foreignKeyProperties"> The properties that make up the foreign key. </param>
lajones marked this conversation as resolved.
Show resolved Hide resolved
/// <param name="principalKeyProperties"> The corresponding keys on the principal side. </param>
public static void IncompatibleMatchingForeignKeyProperties(
[NotNull] this IDiagnosticsLogger<DbLoggerCategory.Model> diagnostics,
[NotNull] IConventionEntityType dependentEntityType,
[NotNull] IConventionEntityType principalEntityType,
[NotNull] IReadOnlyList<IPropertyBase> foreignKeyProperties,
[NotNull] IReadOnlyList<IPropertyBase> principalKeyProperties)
{
Expand All @@ -1094,15 +1098,19 @@ public static void IncompatibleMatchingForeignKeyProperties(
{
definition.Log(
diagnostics,
dependentEntityType.DisplayName(),
principalEntityType.DisplayName(),
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,
dependentEntityType,
principalEntityType,
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.DependentEntityType.DisplayName(),
p.PrincipalEntityType.DisplayName(),
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="principalEntityType"> The entity type on the principal end of the relationship. </param>
/// <param name="dependentEntityType"> The entity type on the dependent 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] IConventionEntityType dependentEntityType,
[NotNull] IConventionEntityType principalEntityType,
[NotNull] IReadOnlyList<IPropertyBase> firstPropertyCollection,
[NotNull] IReadOnlyList<IPropertyBase> secondPropertyCollection)
: base(eventDefinition, messageGenerator, firstPropertyCollection, secondPropertyCollection)
{
DependentEntityType = dependentEntityType;
PrincipalEntityType = principalEntityType;
}

/// <summary>
/// The dependent entity type.
/// </summary>
public virtual IConventionEntityType DependentEntityType { get; }

/// <summary>
/// The principal entity type.
/// </summary>
public virtual IConventionEntityType PrincipalEntityType { get; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,8 @@ private bool TryFindMatchingProperties(
p => !p.IsShadowProperty()
|| p.GetConfigurationSource().Overrides(ConfigurationSource.DataAnnotation)))
{
Dependencies.Logger.IncompatibleMatchingForeignKeyProperties(foreignKeyProperties, propertiesToReference);
Dependencies.Logger.IncompatibleMatchingForeignKeyProperties(
dependentEntityType, principalEntityType, 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 entity type '{dependentEntityType}' and principal entity type '{principalEntityType}', 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 @@ -507,6 +507,7 @@ public void Does_not_match_principal_type_plus_PK_name_property_of_different_typ
Assert.Equal(LogLevel.Debug, logEntry.Level);
Assert.Equal(
CoreResources.LogIncompatibleMatchingForeignKeyProperties(new TestLogger<TestLoggingDefinitions>()).GenerateMessage(
nameof(DependentEntity), nameof(PrincipalEntity),
"{'PrincipalEntityPeeKay' : string}", "{'PeeKay' : int}"), logEntry.Message);

ValidateModel();
Expand Down