Skip to content

Commit

Permalink
Log warnings when RequiredAttribute or non-nullable references are ig…
Browse files Browse the repository at this point in the history 
…nored.

Resolves #15686
  • Loading branch information
@AndriySvyryd
AndriySvyryd committed Aug 20, 2019
1 parent eb58569 commit 346cc83
Show file tree
Hide file tree
Showing 10 changed files with 344 additions and 65 deletions.
52 changes: 47 additions & 5 deletions src/EFCore/Diagnostics/CoreEventId.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) .NET Foundation. All rights reserved.
// 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;
Expand Down Expand Up @@ -85,9 +85,7 @@ private enum Id
RedundantIndexRemoved,
IncompatibleMatchingForeignKeyProperties,
RequiredAttributeOnDependent,
NonNullableOnDependent,
RequiredAttributeOnBothNavigations,
NonNullableReferenceOnBothNavigations,
ConflictingShadowForeignKeysWarning,
MultiplePrimaryKeyCandidates,
MultipleNavigationProperties,
Expand All @@ -98,6 +96,11 @@ private enum Id
ForeignKeyAttributesOnBothNavigationsWarning,
ConflictingForeignKeyAttributesOnNavigationAndPropertyWarning,
RedundantForeignKeyWarning,
NonNullableInverted,
NonNullableReferenceOnBothNavigations,
NonNullableReferenceOnDependent,
RequiredAttributeInverted,
RequiredAttributeOnCollection,

// ChangeTracking events
DetectChangesStarting = CoreBaseId + 800,
Expand Down Expand Up @@ -435,7 +438,7 @@ public static readonly EventId PossibleUnintendedReferenceComparisonWarning
/// This event uses the <see cref="NavigationEventData" /> payload when used with a <see cref="DiagnosticSource" />.
/// </para>
/// </summary>
public static readonly EventId RequiredAttributeOnDependent = MakeModelId(Id.RequiredAttributeOnDependent);
public static readonly EventId RequiredAttributeInverted = MakeModelId(Id.RequiredAttributeInverted);

/// <summary>
/// <para>
Expand All @@ -449,7 +452,7 @@ public static readonly EventId PossibleUnintendedReferenceComparisonWarning
/// This event uses the <see cref="NavigationEventData" /> payload when used with a <see cref="DiagnosticSource" />.
/// </para>
/// </summary>
public static readonly EventId NonNullableOnDependent = MakeModelId(Id.NonNullableOnDependent);
public static readonly EventId NonNullableInverted = MakeModelId(Id.NonNullableInverted);

/// <summary>
/// <para>
Expand Down Expand Up @@ -477,6 +480,45 @@ public static readonly EventId PossibleUnintendedReferenceComparisonWarning
/// </summary>
public static readonly EventId NonNullableReferenceOnBothNavigations = MakeModelId(Id.NonNullableReferenceOnBothNavigations);

/// <summary>
/// <para>
/// The <see cref="RequiredAttribute" /> on the navigation property to the dependent entity was ignored.
/// </para>
/// <para>
/// This event is in the <see cref="DbLoggerCategory.Model" /> category.
/// </para>
/// <para>
/// This event uses the <see cref="NavigationEventData" /> payload when used with a <see cref="DiagnosticSource" />.
/// </para>
/// </summary>
public static readonly EventId RequiredAttributeOnDependent = MakeModelId(Id.RequiredAttributeOnDependent);

/// <summary>
/// <para>
/// The non-nullability of the navigation property to the dependent entity was ignored.
/// </para>
/// <para>
/// This event is in the <see cref="DbLoggerCategory.Model" /> category.
/// </para>
/// <para>
/// This event uses the <see cref="NavigationEventData" /> payload when used with a <see cref="DiagnosticSource" />.
/// </para>
/// </summary>
public static readonly EventId NonNullableReferenceOnDependent = MakeModelId(Id.NonNullableReferenceOnDependent);

/// <summary>
/// <para>
/// The <see cref="RequiredAttribute" /> on the collection navigation property was ignored.
/// </para>
/// <para>
/// This event is in the <see cref="DbLoggerCategory.Model" /> category.
/// </para>
/// <para>
/// This event uses the <see cref="NavigationEventData" /> payload when used with a <see cref="DiagnosticSource" />.
/// </para>
/// </summary>
public static readonly EventId RequiredAttributeOnCollection = MakeModelId(Id.RequiredAttributeOnCollection);

/// <summary>
/// <para>
/// The properties that best match the foreign key convention are already used by a different foreign key.
Expand Down
134 changes: 124 additions & 10 deletions src/EFCore/Diagnostics/CoreLoggerExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1112,15 +1112,15 @@ private static string IncompatibleMatchingForeignKeyProperties(EventDefinitionBa
}

/// <summary>
/// Logs for the <see cref="CoreEventId.RequiredAttributeOnDependent" /> event.
/// Logs for the <see cref="CoreEventId.RequiredAttributeInverted" /> event.
/// </summary>
/// <param name="diagnostics"> The diagnostics logger to use. </param>
/// <param name="navigation"> The navigation property. </param>
public static void RequiredAttributeOnDependent(
public static void RequiredAttributeInverted(
[NotNull] this IDiagnosticsLogger<DbLoggerCategory.Model> diagnostics,
[NotNull] INavigation navigation)
{
var definition = CoreResources.LogRequiredAttributeOnDependent(diagnostics);
var definition = CoreResources.LogRequiredAttributeInverted(diagnostics);

var warningBehavior = definition.GetLogBehavior(diagnostics);
if (warningBehavior != WarningBehavior.Ignore)
Expand All @@ -1137,28 +1137,28 @@ public static void RequiredAttributeOnDependent(
definition.EventId.Name,
new NavigationEventData(
definition,
RequiredAttributeOnDependent,
RequiredAttributeInverted,
navigation));
}
}

private static string RequiredAttributeOnDependent(EventDefinitionBase definition, EventData payload)
private static string RequiredAttributeInverted(EventDefinitionBase definition, EventData payload)
{
var d = (EventDefinition<string, string>)definition;
var p = (NavigationEventData)payload;
return d.GenerateMessage(p.Navigation.Name, p.Navigation.DeclaringEntityType.DisplayName());
}

/// <summary>
/// Logs for the <see cref="CoreEventId.NonNullableOnDependent" /> event.
/// Logs for the <see cref="CoreEventId.NonNullableInverted" /> event.
/// </summary>
/// <param name="diagnostics"> The diagnostics logger to use. </param>
/// <param name="navigation"> The navigation property. </param>
public static void NonNullableOnDependent(
public static void NonNullableInverted(
[NotNull] this IDiagnosticsLogger<DbLoggerCategory.Model> diagnostics,
[NotNull] INavigation navigation)
{
var definition = CoreResources.LogNonNullableOnDependent(diagnostics);
var definition = CoreResources.LogNonNullableInverted(diagnostics);

var warningBehavior = definition.GetLogBehavior(diagnostics);
if (warningBehavior != WarningBehavior.Ignore)
Expand All @@ -1175,12 +1175,12 @@ public static void NonNullableOnDependent(
definition.EventId.Name,
new NavigationEventData(
definition,
NonNullableOnDependent,
NonNullableInverted,
navigation));
}
}

private static string NonNullableOnDependent(EventDefinitionBase definition, EventData payload)
private static string NonNullableInverted(EventDefinitionBase definition, EventData payload)
{
var d = (EventDefinition<string, string>)definition;
var p = (NavigationEventData)payload;
Expand Down Expand Up @@ -1299,6 +1299,120 @@ private static string NonNullableReferenceOnBothNavigations(EventDefinitionBase
secondNavigation.Name);
}

/// <summary>
/// Logs for the <see cref="CoreEventId.RequiredAttributeOnDependent" /> event.
/// </summary>
/// <param name="diagnostics"> The diagnostics logger to use. </param>
/// <param name="navigation"> The navigation property. </param>
public static void RequiredAttributeOnDependent(
[NotNull] this IDiagnosticsLogger<DbLoggerCategory.Model> diagnostics,
[NotNull] INavigation navigation)
{
var definition = CoreResources.LogRequiredAttributeOnDependent(diagnostics);

var warningBehavior = definition.GetLogBehavior(diagnostics);
if (warningBehavior != WarningBehavior.Ignore)
{
definition.Log(
diagnostics,
warningBehavior,
navigation.Name, navigation.DeclaringEntityType.DisplayName());
}

if (diagnostics.DiagnosticSource.IsEnabled(definition.EventId.Name))
{
diagnostics.DiagnosticSource.Write(
definition.EventId.Name,
new NavigationEventData(
definition,
RequiredAttributeOnDependent,
navigation));
}
}

private static string RequiredAttributeOnDependent(EventDefinitionBase definition, EventData payload)
{
var d = (EventDefinition<string, string>)definition;
var p = (NavigationEventData)payload;
return d.GenerateMessage(p.Navigation.Name, p.Navigation.DeclaringEntityType.DisplayName());
}

/// <summary>
/// Logs for the <see cref="CoreEventId.NonNullableReferenceOnDependent" /> event.
/// </summary>
/// <param name="diagnostics"> The diagnostics logger to use. </param>
/// <param name="navigation"> The navigation property. </param>
public static void NonNullableReferenceOnDependent(
[NotNull] this IDiagnosticsLogger<DbLoggerCategory.Model> diagnostics,
[NotNull] INavigation navigation)
{
var definition = CoreResources.LogNonNullableReferenceOnDependent(diagnostics);

var warningBehavior = definition.GetLogBehavior(diagnostics);
if (warningBehavior != WarningBehavior.Ignore)
{
definition.Log(
diagnostics,
warningBehavior,
navigation.Name, navigation.DeclaringEntityType.DisplayName());
}

if (diagnostics.DiagnosticSource.IsEnabled(definition.EventId.Name))
{
diagnostics.DiagnosticSource.Write(
definition.EventId.Name,
new NavigationEventData(
definition,
NonNullableReferenceOnDependent,
navigation));
}
}

private static string NonNullableReferenceOnDependent(EventDefinitionBase definition, EventData payload)
{
var d = (EventDefinition<string, string>)definition;
var p = (NavigationEventData)payload;
return d.GenerateMessage(p.Navigation.Name, p.Navigation.DeclaringEntityType.DisplayName());
}

/// <summary>
/// Logs for the <see cref="CoreEventId.RequiredAttributeOnCollection" /> event.
/// </summary>
/// <param name="diagnostics"> The diagnostics logger to use. </param>
/// <param name="navigation"> The navigation property. </param>
public static void RequiredAttributeOnCollection(
[NotNull] this IDiagnosticsLogger<DbLoggerCategory.Model> diagnostics,
[NotNull] INavigation navigation)
{
var definition = CoreResources.LogRequiredAttributeOnCollection(diagnostics);

var warningBehavior = definition.GetLogBehavior(diagnostics);
if (warningBehavior != WarningBehavior.Ignore)
{
definition.Log(
diagnostics,
warningBehavior,
navigation.Name, navigation.DeclaringEntityType.DisplayName());
}

if (diagnostics.DiagnosticSource.IsEnabled(definition.EventId.Name))
{
diagnostics.DiagnosticSource.Write(
definition.EventId.Name,
new NavigationEventData(
definition,
RequiredAttributeOnCollection,
navigation));
}
}

private static string RequiredAttributeOnCollection(EventDefinitionBase definition, EventData payload)
{
var d = (EventDefinition<string, string>)definition;
var p = (NavigationEventData)payload;
return d.GenerateMessage(p.Navigation.Name, p.Navigation.DeclaringEntityType.DisplayName());
}

/// <summary>
/// Logs for the <see cref="CoreEventId.ConflictingShadowForeignKeysWarning" /> event.
/// </summary>
Expand Down
29 changes: 28 additions & 1 deletion src/EFCore/Diagnostics/LoggingDefinitions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,24 @@ public abstract class LoggingDefinitions
[EntityFrameworkInternal]
public EventDefinitionBase LogIncompatibleMatchingForeignKeyProperties;

/// <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 LogRequiredAttributeInverted;

/// <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 LogNonNullableInverted;

/// <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 All @@ -464,7 +482,7 @@ public abstract class LoggingDefinitions
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
[EntityFrameworkInternal]
public EventDefinitionBase LogNonNullableOnDependent;
public EventDefinitionBase LogNonNullableReferenceOnDependent;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down Expand Up @@ -493,6 +511,15 @@ public abstract class LoggingDefinitions
[EntityFrameworkInternal]
public EventDefinitionBase LogForeignKeyAttributesOnBothNavigations;

/// <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 LogRequiredAttributeOnCollection;

/// <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
3 changes: 2 additions & 1 deletion src/EFCore/Metadata/Conventions/NonNullableNavigationConvention.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ public virtual void ProcessNavigationAdded(
if (!navigation.ForeignKey.IsUnique
|| relationshipBuilder.Metadata.GetPrincipalEndConfigurationSource() != null)
{
Dependencies.Logger.NonNullableReferenceOnDependent(navigation.ForeignKey.DependentToPrincipal);
return;
}

Expand All @@ -74,7 +75,7 @@ public virtual void ProcessNavigationAdded(
return;
}

Dependencies.Logger.NonNullableOnDependent(newRelationshipBuilder.Metadata.DependentToPrincipal);
Dependencies.Logger.NonNullableInverted(newRelationshipBuilder.Metadata.DependentToPrincipal);
relationshipBuilder = newRelationshipBuilder;
}

Expand Down
7 changes: 4 additions & 3 deletions src/EFCore/Metadata/Conventions/RequiredNavigationAttributeConvention.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ public override void ProcessNavigationAdded(

if (navigation.IsCollection())
{
Dependencies.Logger.RequiredAttributeOnCollection(navigation.ForeignKey.DependentToPrincipal);
return;
}

Expand All @@ -61,9 +62,9 @@ public override void ProcessNavigationAdded(
}
}

if (!navigation.ForeignKey.IsUnique
|| relationshipBuilder.Metadata.GetPrincipalEndConfigurationSource() != null)
if (relationshipBuilder.Metadata.GetPrincipalEndConfigurationSource() != null)
{
Dependencies.Logger.RequiredAttributeOnDependent(navigation.ForeignKey.DependentToPrincipal);
return;
}

Expand All @@ -76,7 +77,7 @@ public override void ProcessNavigationAdded(
return;
}

Dependencies.Logger.RequiredAttributeOnDependent(newRelationshipBuilder.Metadata.DependentToPrincipal);
Dependencies.Logger.RequiredAttributeInverted(navigation.ForeignKey.DependentToPrincipal);
relationshipBuilder = newRelationshipBuilder;
}

Expand Down
Loading

0 comments on commit 346cc83

Please sign in to comment.