Skip to content

Commit

Permalink
Warn when entity type, property, or navigation is ignored after being…
Browse files Browse the repository at this point in the history
… explicitly mapped

An old bug.

Fixes #12082
  • Loading branch information
ajcvickers committed Dec 23, 2022
1 parent 06a41bb commit 0348629
Show file tree
Hide file tree
Showing 14 changed files with 485 additions and 31 deletions.
54 changes: 54 additions & 0 deletions src/EFCore/Diagnostics/CoreEventId.cs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,9 @@ private enum Id
RequiredAttributeOnSkipNavigation,
AmbiguousEndRequiredWarning,
ShadowForeignKeyPropertyCreated,
MappedEntityTypeIgnoredWarning,
MappedNavigationIgnoredWarning,
MappedPropertyIgnoredWarning,

// ChangeTracking events
DetectChangesStarting = CoreBaseId + 800,
Expand Down Expand Up @@ -512,6 +515,57 @@ private static EventId MakeModelValidationId(Id id)
/// </remarks>
public static readonly EventId ShadowForeignKeyPropertyCreated = MakeModelValidationId(Id.ShadowForeignKeyPropertyCreated);

/// <summary>
/// An entity type was first mapped explicitly and then ignored.
/// </summary>
/// <remarks>
/// <para>
/// This event is in the <see cref="DbLoggerCategory.Model" /> category.
/// </para>
/// <para>
/// This event uses the <see cref="EntityTypeEventData" /> payload when used with a <see cref="DiagnosticSource" />.
/// </para>
/// <para>
/// See <see href="https://aka.ms/efcore-docs-modeling">Modeling entity types and relationships</see> for more information and
/// examples.
/// </para>
/// </remarks>
public static readonly EventId MappedEntityTypeIgnoredWarning = MakeModelId(Id.MappedEntityTypeIgnoredWarning);

/// <summary>
/// A navigation was first mapped explicitly and then ignored.
/// </summary>
/// <remarks>
/// <para>
/// This event is in the <see cref="DbLoggerCategory.Model" /> category.
/// </para>
/// <para>
/// This event uses the <see cref="NavigationBaseEventData" /> payload when used with a <see cref="DiagnosticSource" />.
/// </para>
/// <para>
/// See <see href="https://aka.ms/efcore-docs-modeling">Modeling entity types and relationships</see> for more information and
/// examples.
/// </para>
/// </remarks>
public static readonly EventId MappedNavigationIgnoredWarning = MakeModelId(Id.MappedNavigationIgnoredWarning);

/// <summary>
/// A property was first mapped explicitly and then ignored.
/// </summary>
/// <remarks>
/// <para>
/// This event is in the <see cref="DbLoggerCategory.Model" /> category.
/// </para>
/// <para>
/// This event uses the <see cref="PropertyEventData" /> payload when used with a <see cref="DiagnosticSource" />.
/// </para>
/// <para>
/// See <see href="https://aka.ms/efcore-docs-modeling">Modeling entity types and relationships</see> for more information and
/// examples.
/// </para>
/// </remarks>
public static readonly EventId MappedPropertyIgnoredWarning = MakeModelId(Id.MappedPropertyIgnoredWarning);

/// <summary>
/// An index was not created as the properties are already covered.
/// </summary>
Expand Down
93 changes: 93 additions & 0 deletions src/EFCore/Diagnostics/CoreLoggerExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3011,6 +3011,99 @@ private static string CascadeDeleteOrphan(EventDefinitionBase definition, EventD
p.ParentEntityType.ShortName());
}

/// <summary>
/// Logs for the <see cref="CoreEventId.MappedNavigationIgnoredWarning" /> event.
/// </summary>
/// <param name="diagnostics">The diagnostics logger to use.</param>
/// <param name="navigation">The navigation.</param>
public static void MappedNavigationIgnoredWarning(
this IDiagnosticsLogger<DbLoggerCategory.Model> diagnostics,
INavigationBase navigation)
{
var definition = CoreResources.LogMappedNavigationIgnored(diagnostics);

if (diagnostics.ShouldLog(definition))
{
definition.Log(diagnostics, navigation.DeclaringType.ShortName(), navigation.Name);
}

if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
{
var eventData = new NavigationBaseEventData(definition, MappedNavigationIgnoredWarning, navigation);

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

private static string MappedNavigationIgnoredWarning(EventDefinitionBase definition, EventData payload)
{
var d = (EventDefinition<string, string>)definition;
var p = (NavigationBaseEventData)payload;
return d.GenerateMessage(p.NavigationBase.DeclaringType.ShortName(), p.NavigationBase.Name);
}

/// <summary>
/// Logs for the <see cref="CoreEventId.MappedPropertyIgnoredWarning" /> event.
/// </summary>
/// <param name="diagnostics">The diagnostics logger to use.</param>
/// <param name="property">The property.</param>
public static void MappedPropertyIgnoredWarning(
this IDiagnosticsLogger<DbLoggerCategory.Model> diagnostics,
IProperty property)
{
var definition = CoreResources.LogMappedPropertyIgnored(diagnostics);

if (diagnostics.ShouldLog(definition))
{
definition.Log(diagnostics, property.DeclaringType.ShortName(), property.Name);
}

if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
{
var eventData = new PropertyEventData(definition, MappedPropertyIgnoredWarning, property);

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

private static string MappedPropertyIgnoredWarning(EventDefinitionBase definition, EventData payload)
{
var d = (EventDefinition<string, string>)definition;
var p = (PropertyEventData)payload;
return d.GenerateMessage(p.Property.DeclaringType.ShortName(), p.Property.Name);
}

/// <summary>
/// Logs for the <see cref="CoreEventId.MappedEntityTypeIgnoredWarning" /> event.
/// </summary>
/// <param name="diagnostics">The diagnostics logger to use.</param>
/// <param name="entityType">The entity type.</param>
public static void MappedEntityTypeIgnoredWarning(
this IDiagnosticsLogger<DbLoggerCategory.Model> diagnostics,
IEntityType entityType)
{
var definition = CoreResources.LogMappedEntityTypeIgnored(diagnostics);

if (diagnostics.ShouldLog(definition))
{
definition.Log(diagnostics, entityType.ShortName());
}

if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
{
var eventData = new EntityTypeEventData(definition, MappedEntityTypeIgnoredWarning, entityType);

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

private static string MappedEntityTypeIgnoredWarning(EventDefinitionBase definition, EventData payload)
{
var d = (EventDefinition<string>)definition;
var p = (EntityTypeEventData)payload;
return d.GenerateMessage(p.EntityType.ShortName());
}

/// <summary>
/// Logs for the <see cref="CoreEventId.CascadeDeleteOrphan" /> event.
/// </summary>
Expand Down
27 changes: 27 additions & 0 deletions src/EFCore/Diagnostics/LoggingDefinitions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,33 @@ public abstract class LoggingDefinitions
[EntityFrameworkInternal]
public EventDefinitionBase? LogManyServiceProvidersCreated;

/// <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? LogMappedEntityTypeIgnored;

/// <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? LogMappedNavigationIgnored;

/// <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? LogMappedPropertyIgnored;

/// <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
15 changes: 15 additions & 0 deletions src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1181,6 +1181,11 @@ public virtual bool IsIgnored(string name, ConfigurationSource? configurationSou
var foreignKey = navigation.ForeignKey;
Check.DebugAssert(navigation.DeclaringEntityType == Metadata, "navigation.DeclaringEntityType != Metadata");

if (navigation.GetConfigurationSource() == ConfigurationSource.Explicit)
{
ModelBuilder.Metadata.ScopedModelDependencies?.Logger.MappedNavigationIgnoredWarning(navigation);
}

var navigationConfigurationSource = navigation.GetConfigurationSource();
if ((navigation.IsOnDependent
&& foreignKey.IsOwnership)
Expand Down Expand Up @@ -1211,6 +1216,11 @@ public virtual bool IsIgnored(string name, ConfigurationSource? configurationSou
{
Check.DebugAssert(property.DeclaringEntityType == Metadata, "property.DeclaringEntityType != Metadata");

if (property.GetConfigurationSource() == ConfigurationSource.Explicit)
{
ModelBuilder.Metadata.ScopedModelDependencies?.Logger.MappedPropertyIgnoredWarning(property);
}

var removedProperty = RemoveProperty(property, configurationSource);

Check.DebugAssert(removedProperty != null, "removedProperty is null");
Expand All @@ -1230,6 +1240,11 @@ public virtual bool IsIgnored(string name, ConfigurationSource? configurationSou
Check.DebugAssert(
skipNavigation.DeclaringEntityType == Metadata, "skipNavigation.DeclaringEntityType != Metadata");

if (skipNavigation.GetConfigurationSource() == ConfigurationSource.Explicit)
{
ModelBuilder.Metadata.ScopedModelDependencies?.Logger.MappedNavigationIgnoredWarning(skipNavigation);
}

Metadata.Builder.HasNoSkipNavigation(skipNavigation, configurationSource);
}
else
Expand Down
9 changes: 5 additions & 4 deletions src/EFCore/Metadata/Internal/InternalModelBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,11 @@ public virtual bool CanBeConfigured(
var entityType = Metadata.FindEntityType(name);
if (entityType != null)
{
if (entityType.GetConfigurationSource() == ConfigurationSource.Explicit)
{
Metadata.ScopedModelDependencies?.Logger.MappedEntityTypeIgnoredWarning(entityType);
}

HasNoEntityType(entityType, configurationSource);
}

Expand All @@ -479,10 +484,6 @@ public virtual bool CanBeConfigured(
else
{
Metadata.AddIgnored(type.Type, configurationSource);
}

if (type.Type != null)
{
Metadata.RemoveOwned(type.Type);
}

Expand Down
75 changes: 75 additions & 0 deletions src/EFCore/Properties/CoreStrings.Designer.cs

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

14 changes: 13 additions & 1 deletion src/EFCore/Properties/CoreStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -817,6 +817,18 @@
<value>More than twenty 'IServiceProvider' instances have been created for internal use by Entity Framework. This is commonly caused by injection of a new singleton service instance into every DbContext instance. For example, calling 'UseLoggerFactory' passing in a new instance each time--see https://go.microsoft.com/fwlink/?linkid=869049 for more details. This may lead to performance issues, consider reviewing calls on 'DbContextOptionsBuilder' that may require new service providers to be built.</value>
<comment>Warning CoreEventId.ManyServiceProvidersCreatedWarning</comment>
</data>
<data name="LogMappedEntityTypeIgnored" xml:space="preserve">
<value>The entity type '{entityType}' was first mapped explicitly and then ignored. Consider not mapping the entity type in the first place.</value>
<comment>Warning CoreEventId.MappedEntityTypeIgnored string</comment>
</data>
<data name="LogMappedPropertyIgnored" xml:space="preserve">
<value>The property '{entityType}.{property}' was first mapped explicitly and then ignored. Consider not mapping the property in the first place.</value>
<comment>Warning CoreEventId.MappedPropertyIgnored string string</comment>
</data>
<data name="LogMappedNavigationIgnored" xml:space="preserve">
<value>The navigation '{entityType}.{navigation}' was first mapped explicitly and then ignored. Consider not mapping the navigation in the first place.</value>
<comment>Warning CoreEventId.MappedNavigationIgnored string string</comment>
</data>
<data name="LogMultipleInversePropertiesSameTarget" xml:space="preserve">
<value>There are multiple navigations ({navigations}) configured with [InverseProperty] attribute which point to the same inverse navigation '{inverseNavigation}' therefore no relationship was configured by convention.</value>
<comment>Warning CoreEventId.MultipleInversePropertiesSameTargetWarning string string?</comment>
Expand Down Expand Up @@ -1508,4 +1520,4 @@
<data name="WrongStateManager" xml:space="preserve">
<value>Cannot start tracking the entry for entity type '{entityType}' because it was created by a different StateManager instance.</value>
</data>
</root>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@ protected override string StoreName
=> "InheritanceBulkUpdatesTest";

public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder)
=> base.AddOptions(builder).ConfigureWarnings(w => w.Log(CoreEventId.FirstWithoutOrderByAndFilterWarning));
=> base.AddOptions(builder).ConfigureWarnings(
w => w.Log(CoreEventId.FirstWithoutOrderByAndFilterWarning)
.Ignore(
CoreEventId.MappedEntityTypeIgnoredWarning,
CoreEventId.MappedPropertyIgnoredWarning,
CoreEventId.MappedNavigationIgnoredWarning));

public void UseTransaction(DatabaseFacade facade, IDbContextTransaction transaction)
=> facade.UseTransaction(transaction.GetDbTransaction());
Expand Down
Loading

0 comments on commit 0348629

Please sign in to comment.