Skip to content

Commit

Permalink
Log an error for Required on principal to dependent
Browse files Browse the repository at this point in the history
Fixes #17286
  • Loading branch information
AndriySvyryd committed Jul 20, 2020
1 parent 644d3c8 commit 8e2832a
Show file tree
Hide file tree
Showing 12 changed files with 163 additions and 274 deletions.
32 changes: 2 additions & 30 deletions src/EFCore/Diagnostics/CoreEventId.cs
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,10 @@ private enum Id
ForeignKeyAttributesOnBothNavigationsWarning,
ConflictingForeignKeyAttributesOnNavigationAndPropertyWarning,
RedundantForeignKeyWarning,
NonNullableInverted,
NonNullableInverted, // Unused
NonNullableReferenceOnBothNavigations,
NonNullableReferenceOnDependent,
RequiredAttributeInverted,
RequiredAttributeInverted, // Unused
RequiredAttributeOnCollection,
CollectionWithoutComparer,
ConflictingKeylessAndKeyAttributesWarning,
Expand Down Expand Up @@ -393,34 +393,6 @@ private enum Id
/// </summary>
public static readonly EventId IncompatibleMatchingForeignKeyProperties = MakeModelId(Id.IncompatibleMatchingForeignKeyProperties);

/// <summary>
/// <para>
/// The entity type with the navigation property that has the <see cref="RequiredAttribute" />
/// was configured as the dependent side in the relationship.
/// </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 RequiredAttributeInverted = MakeModelId(Id.RequiredAttributeInverted);

/// <summary>
/// <para>
/// The entity type with the navigation property that has non-nullability
/// was configured as the dependent side in the relationship.
/// </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 NonNullableInverted = MakeModelId(Id.NonNullableInverted);

/// <summary>
/// <para>
/// Navigations separated into two relationships as <see cref="RequiredAttribute" /> was specified on both navigations.
Expand Down
78 changes: 5 additions & 73 deletions src/EFCore/Diagnostics/CoreLoggerExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1167,74 +1167,6 @@ private static string IncompatibleMatchingForeignKeyProperties(EventDefinitionBa
p.SecondPropertyCollection.Format(includeTypes: true));
}

/// <summary>
/// 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 RequiredAttributeInverted(
[NotNull] this IDiagnosticsLogger<DbLoggerCategory.Model> diagnostics,
[NotNull] INavigation navigation)
{
var definition = CoreResources.LogRequiredAttributeInverted(diagnostics);

if (diagnostics.ShouldLog(definition))
{
definition.Log(diagnostics,navigation.Name, navigation.DeclaringEntityType.DisplayName());
}

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

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

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.NonNullableInverted" /> event.
/// </summary>
/// <param name="diagnostics"> The diagnostics logger to use. </param>
/// <param name="navigation"> The navigation property. </param>
public static void NonNullableInverted(
[NotNull] this IDiagnosticsLogger<DbLoggerCategory.Model> diagnostics,
[NotNull] INavigation navigation)
{
var definition = CoreResources.LogNonNullableInverted(diagnostics);

if (diagnostics.ShouldLog(definition))
{
definition.Log(diagnostics,navigation.Name, navigation.DeclaringEntityType.DisplayName());
}

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

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

private static string NonNullableInverted(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.RequiredAttributeOnBothNavigations" /> event.
/// </summary>
Expand Down Expand Up @@ -1346,7 +1278,7 @@ private static string NonNullableReferenceOnBothNavigations(EventDefinitionBase
{
definition.Log(
diagnostics,
navigation.Name, navigation.DeclaringEntityType.DisplayName());
navigation.DeclaringEntityType.DisplayName(), navigation.Name);
}

if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
Expand All @@ -1364,7 +1296,7 @@ private static string RequiredAttributeOnDependent(EventDefinitionBase definitio
{
var d = (EventDefinition<string, string>)definition;
var p = (NavigationEventData)payload;
return d.GenerateMessage(p.Navigation.Name, p.Navigation.DeclaringEntityType.DisplayName());
return d.GenerateMessage(p.Navigation.DeclaringEntityType.DisplayName(), p.Navigation.Name);
}

/// <summary>
Expand All @@ -1382,8 +1314,8 @@ private static string RequiredAttributeOnDependent(EventDefinitionBase definitio
{
definition.Log(
diagnostics,
navigation.Name,
navigation.DeclaringEntityType.DisplayName());
navigation.DeclaringEntityType.DisplayName(),
navigation.Name);
}

if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
Expand All @@ -1401,7 +1333,7 @@ private static string NonNullableReferenceOnDependent(EventDefinitionBase defini
{
var d = (EventDefinition<string, string>)definition;
var p = (NavigationEventData)payload;
return d.GenerateMessage(p.Navigation.Name, p.Navigation.DeclaringEntityType.DisplayName());
return d.GenerateMessage(p.Navigation.DeclaringEntityType.DisplayName(), p.Navigation.Name);
}

/// <summary>
Expand Down
18 changes: 0 additions & 18 deletions src/EFCore/Diagnostics/LoggingDefinitions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -493,24 +493,6 @@ 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 Down
3 changes: 2 additions & 1 deletion src/EFCore/Infrastructure/CoreOptionsExtension.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ private WarningsConfiguration _warningsConfiguration
.TryWithExplicit(CoreEventId.ManyServiceProvidersCreatedWarning, WarningBehavior.Throw)
.TryWithExplicit(CoreEventId.LazyLoadOnDisposedContextWarning, WarningBehavior.Throw)
.TryWithExplicit(CoreEventId.DetachedLazyLoadingWarning, WarningBehavior.Throw)
.TryWithExplicit(CoreEventId.InvalidIncludePathError, WarningBehavior.Throw);
.TryWithExplicit(CoreEventId.InvalidIncludePathError, WarningBehavior.Throw)
.TryWithExplicit(CoreEventId.RequiredAttributeOnDependent, WarningBehavior.Throw);

/// <summary>
/// Creates a new set of options with everything set to default values.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ public virtual ConventionSet CreateConventionSet()
var databaseGeneratedAttributeConvention = new DatabaseGeneratedAttributeConvention(Dependencies);
var requiredPropertyAttributeConvention = new RequiredPropertyAttributeConvention(Dependencies);
var nonNullableReferencePropertyConvention = new NonNullableReferencePropertyConvention(Dependencies);
var nonNullableNavigationConvention = new NonNullableNavigationConvention(Dependencies);
var maxLengthAttributeConvention = new MaxLengthAttributeConvention(Dependencies);
var stringLengthAttributeConvention = new StringLengthAttributeConvention(Dependencies);
var timestampAttributeConvention = new TimestampAttributeConvention(Dependencies);
Expand Down Expand Up @@ -169,29 +168,11 @@ public virtual ConventionSet CreateConventionSet()
conventionSet.ForeignKeyOwnershipChangedConventions.Add(relationshipDiscoveryConvention);
conventionSet.ForeignKeyOwnershipChangedConventions.Add(valueGeneratorConvention);

conventionSet.ModelInitializedConventions.Add(new DbSetFindingConvention(Dependencies));

conventionSet.ModelFinalizingConventions.Add(new ModelCleanupConvention(Dependencies));
conventionSet.ModelFinalizingConventions.Add(keyAttributeConvention);
conventionSet.ModelFinalizingConventions.Add(indexAttributeConvention);
conventionSet.ModelFinalizingConventions.Add(foreignKeyAttributeConvention);
conventionSet.ModelFinalizingConventions.Add(new ChangeTrackingStrategyConvention(Dependencies));
conventionSet.ModelFinalizingConventions.Add(new ConstructorBindingConvention(Dependencies));
conventionSet.ModelFinalizingConventions.Add(new TypeMappingConvention(Dependencies));
conventionSet.ModelFinalizingConventions.Add(foreignKeyIndexConvention);
conventionSet.ModelFinalizingConventions.Add(foreignKeyPropertyDiscoveryConvention);
conventionSet.ModelFinalizingConventions.Add(servicePropertyDiscoveryConvention);
conventionSet.ModelFinalizingConventions.Add(nonNullableReferencePropertyConvention);
conventionSet.ModelFinalizingConventions.Add(nonNullableNavigationConvention);
conventionSet.ModelFinalizingConventions.Add(new QueryFilterRewritingConvention(Dependencies));
conventionSet.ModelFinalizingConventions.Add(inversePropertyAttributeConvention);
conventionSet.ModelFinalizingConventions.Add(backingFieldConvention);

conventionSet.ModelFinalizedConventions.Add(new ValidatingConvention(Dependencies));

var requiredNavigationAttributeConvention = new RequiredNavigationAttributeConvention(Dependencies);
var nonNullableNavigationConvention = new NonNullableNavigationConvention(Dependencies);
conventionSet.NavigationAddedConventions.Add(backingFieldConvention);
conventionSet.NavigationAddedConventions.Add(new NavigationBackingFieldAttributeConvention(Dependencies));
conventionSet.NavigationAddedConventions.Add(new RequiredNavigationAttributeConvention(Dependencies));
conventionSet.NavigationAddedConventions.Add(requiredNavigationAttributeConvention);
conventionSet.NavigationAddedConventions.Add(nonNullableNavigationConvention);
conventionSet.NavigationAddedConventions.Add(inversePropertyAttributeConvention);
conventionSet.NavigationAddedConventions.Add(foreignKeyPropertyDiscoveryConvention);
Expand All @@ -211,6 +192,8 @@ public virtual ConventionSet CreateConventionSet()
conventionSet.IndexUniquenessChangedConventions.Add(foreignKeyIndexConvention);

conventionSet.ForeignKeyPrincipalEndChangedConventions.Add(foreignKeyPropertyDiscoveryConvention);
conventionSet.ForeignKeyPrincipalEndChangedConventions.Add(requiredNavigationAttributeConvention);
conventionSet.ForeignKeyPrincipalEndChangedConventions.Add(nonNullableNavigationConvention);

conventionSet.PropertyNullabilityChangedConventions.Add(foreignKeyPropertyDiscoveryConvention);

Expand All @@ -224,6 +207,26 @@ public virtual ConventionSet CreateConventionSet()
conventionSet.PropertyFieldChangedConventions.Add(stringLengthAttributeConvention);
conventionSet.PropertyFieldChangedConventions.Add(timestampAttributeConvention);

conventionSet.ModelInitializedConventions.Add(new DbSetFindingConvention(Dependencies));

conventionSet.ModelFinalizingConventions.Add(new ModelCleanupConvention(Dependencies));
conventionSet.ModelFinalizingConventions.Add(keyAttributeConvention);
conventionSet.ModelFinalizingConventions.Add(indexAttributeConvention);
conventionSet.ModelFinalizingConventions.Add(foreignKeyAttributeConvention);
conventionSet.ModelFinalizingConventions.Add(new ChangeTrackingStrategyConvention(Dependencies));
conventionSet.ModelFinalizingConventions.Add(new ConstructorBindingConvention(Dependencies));
conventionSet.ModelFinalizingConventions.Add(new TypeMappingConvention(Dependencies));
conventionSet.ModelFinalizingConventions.Add(foreignKeyIndexConvention);
conventionSet.ModelFinalizingConventions.Add(foreignKeyPropertyDiscoveryConvention);
conventionSet.ModelFinalizingConventions.Add(servicePropertyDiscoveryConvention);
conventionSet.ModelFinalizingConventions.Add(nonNullableReferencePropertyConvention);
conventionSet.ModelFinalizingConventions.Add(nonNullableNavigationConvention);
conventionSet.ModelFinalizingConventions.Add(new QueryFilterRewritingConvention(Dependencies));
conventionSet.ModelFinalizingConventions.Add(inversePropertyAttributeConvention);
conventionSet.ModelFinalizingConventions.Add(backingFieldConvention);

conventionSet.ModelFinalizedConventions.Add(new ValidatingConvention(Dependencies));

return conventionSet;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ public abstract class NavigationAttributeConventionBase<TAttribute> :
IEntityTypeAddedConvention,
IEntityTypeIgnoredConvention,
IEntityTypeBaseTypeChangedConvention,
IEntityTypeMemberIgnoredConvention,
INavigationAddedConvention,
IEntityTypeMemberIgnoredConvention
IForeignKeyPrincipalEndChangedConvention
where TAttribute : Attribute
{
/// <summary>
Expand Down Expand Up @@ -170,12 +171,23 @@ protected NavigationAttributeConventionBase([NotNull] ProviderConventionSetBuild
}
}

/// <summary>
/// Called after an entity type member is ignored.
/// </summary>
/// <param name="entityTypeBuilder"> The builder for the entity type. </param>
/// <param name="name"> The name of the ignored member. </param>
/// <param name="context"> Additional information associated with convention execution. </param>
/// <inheritdoc />
public virtual void ProcessForeignKeyPrincipalEndChanged(
IConventionForeignKeyBuilder relationshipBuilder,
IConventionContext<IConventionForeignKeyBuilder> context)
{
var fk = relationshipBuilder.Metadata;
var dependentToPrincipalAttributes = fk.DependentToPrincipal == null
? null
: GetAttributes<TAttribute>(fk.DeclaringEntityType, fk.DependentToPrincipal);
var principalToDependentAttributes = fk.PrincipalToDependent == null
? null
: GetAttributes<TAttribute>(fk.PrincipalEntityType, fk.PrincipalToDependent);
ProcessForeignKeyPrincipalEndChanged(
relationshipBuilder, dependentToPrincipalAttributes, principalToDependentAttributes, context);
}

/// <inheritdoc />
public virtual void ProcessEntityTypeMemberIgnored(
IConventionEntityTypeBuilder entityTypeBuilder, string name, IConventionContext<string> context)
{
Expand Down Expand Up @@ -316,5 +328,19 @@ private Type FindCandidateNavigationWithAttributePropertyType([NotNull] Property
[NotNull] TAttribute attribute,
[NotNull] IConventionContext<string> context)
=> throw new NotImplementedException();

/// <summary>
/// Called after the principal end of a foreign key is changed.
/// </summary>
/// <param name="relationshipBuilder"> The builder for the foreign key. </param>
/// <param name="dependentToPrincipalAttributes"> The attributes on the dependent to principal navigation. </param>
/// <param name="principalToDependentAttributes"> The attributes on the principal to dependent navigation. </param>
/// <param name="context"> Additional information associated with convention execution. </param>
public virtual void ProcessForeignKeyPrincipalEndChanged(
[NotNull] IConventionForeignKeyBuilder relationshipBuilder,
[CanBeNull] IEnumerable<TAttribute> dependentToPrincipalAttributes,
[CanBeNull] IEnumerable<TAttribute> principalToDependentAttributes,
[NotNull] IConventionContext<IConventionForeignKeyBuilder> context)
=> throw new NotImplementedException();
}
}
Loading

0 comments on commit 8e2832a

Please sign in to comment.