diff --git a/src/EFCore/Diagnostics/CoreEventId.cs b/src/EFCore/Diagnostics/CoreEventId.cs index f5bdb484c37..ce45e0e1934 100644 --- a/src/EFCore/Diagnostics/CoreEventId.cs +++ b/src/EFCore/Diagnostics/CoreEventId.cs @@ -97,10 +97,10 @@ private enum Id ForeignKeyAttributesOnBothNavigationsWarning, ConflictingForeignKeyAttributesOnNavigationAndPropertyWarning, RedundantForeignKeyWarning, - NonNullableInverted, + NonNullableInverted, // Unused NonNullableReferenceOnBothNavigations, NonNullableReferenceOnDependent, - RequiredAttributeInverted, + RequiredAttributeInverted, // Unused RequiredAttributeOnCollection, CollectionWithoutComparer, ConflictingKeylessAndKeyAttributesWarning, @@ -393,34 +393,6 @@ private enum Id /// public static readonly EventId IncompatibleMatchingForeignKeyProperties = MakeModelId(Id.IncompatibleMatchingForeignKeyProperties); - /// - /// - /// The entity type with the navigation property that has the - /// was configured as the dependent side in the relationship. - /// - /// - /// This event is in the category. - /// - /// - /// This event uses the payload when used with a . - /// - /// - public static readonly EventId RequiredAttributeInverted = MakeModelId(Id.RequiredAttributeInverted); - - /// - /// - /// The entity type with the navigation property that has non-nullability - /// was configured as the dependent side in the relationship. - /// - /// - /// This event is in the category. - /// - /// - /// This event uses the payload when used with a . - /// - /// - public static readonly EventId NonNullableInverted = MakeModelId(Id.NonNullableInverted); - /// /// /// Navigations separated into two relationships as was specified on both navigations. diff --git a/src/EFCore/Diagnostics/CoreLoggerExtensions.cs b/src/EFCore/Diagnostics/CoreLoggerExtensions.cs index b7d3302c616..2a56894f4f6 100644 --- a/src/EFCore/Diagnostics/CoreLoggerExtensions.cs +++ b/src/EFCore/Diagnostics/CoreLoggerExtensions.cs @@ -1167,74 +1167,6 @@ private static string IncompatibleMatchingForeignKeyProperties(EventDefinitionBa p.SecondPropertyCollection.Format(includeTypes: true)); } - /// - /// Logs for the event. - /// - /// The diagnostics logger to use. - /// The navigation property. - public static void RequiredAttributeInverted( - [NotNull] this IDiagnosticsLogger 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)definition; - var p = (NavigationEventData)payload; - return d.GenerateMessage(p.Navigation.Name, p.Navigation.DeclaringEntityType.DisplayName()); - } - - /// - /// Logs for the event. - /// - /// The diagnostics logger to use. - /// The navigation property. - public static void NonNullableInverted( - [NotNull] this IDiagnosticsLogger 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)definition; - var p = (NavigationEventData)payload; - return d.GenerateMessage(p.Navigation.Name, p.Navigation.DeclaringEntityType.DisplayName()); - } - /// /// Logs for the event. /// @@ -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)) @@ -1364,7 +1296,7 @@ private static string RequiredAttributeOnDependent(EventDefinitionBase definitio { var d = (EventDefinition)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); } /// @@ -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)) @@ -1401,7 +1333,7 @@ private static string NonNullableReferenceOnDependent(EventDefinitionBase defini { var d = (EventDefinition)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); } /// diff --git a/src/EFCore/Diagnostics/LoggingDefinitions.cs b/src/EFCore/Diagnostics/LoggingDefinitions.cs index e68df2c334d..5cfe10236c9 100644 --- a/src/EFCore/Diagnostics/LoggingDefinitions.cs +++ b/src/EFCore/Diagnostics/LoggingDefinitions.cs @@ -493,24 +493,6 @@ public abstract class LoggingDefinitions [EntityFrameworkInternal] public EventDefinitionBase LogIncompatibleMatchingForeignKeyProperties; - /// - /// 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. - /// - [EntityFrameworkInternal] - public EventDefinitionBase LogRequiredAttributeInverted; - - /// - /// 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. - /// - [EntityFrameworkInternal] - public EventDefinitionBase LogNonNullableInverted; - /// /// 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 diff --git a/src/EFCore/Infrastructure/CoreOptionsExtension.cs b/src/EFCore/Infrastructure/CoreOptionsExtension.cs index 2f80ef6cd50..1e339a3e74d 100644 --- a/src/EFCore/Infrastructure/CoreOptionsExtension.cs +++ b/src/EFCore/Infrastructure/CoreOptionsExtension.cs @@ -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); /// /// Creates a new set of options with everything set to default values. diff --git a/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs b/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs index 25be69e5c9e..56457bb2893 100644 --- a/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs +++ b/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs @@ -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); @@ -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); @@ -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); @@ -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; } diff --git a/src/EFCore/Metadata/Conventions/NavigationAttributeConventionBase.cs b/src/EFCore/Metadata/Conventions/NavigationAttributeConventionBase.cs index 121ea2601d2..44a098f64db 100644 --- a/src/EFCore/Metadata/Conventions/NavigationAttributeConventionBase.cs +++ b/src/EFCore/Metadata/Conventions/NavigationAttributeConventionBase.cs @@ -22,8 +22,9 @@ public abstract class NavigationAttributeConventionBase : IEntityTypeAddedConvention, IEntityTypeIgnoredConvention, IEntityTypeBaseTypeChangedConvention, + IEntityTypeMemberIgnoredConvention, INavigationAddedConvention, - IEntityTypeMemberIgnoredConvention + IForeignKeyPrincipalEndChangedConvention where TAttribute : Attribute { /// @@ -170,12 +171,23 @@ protected NavigationAttributeConventionBase([NotNull] ProviderConventionSetBuild } } - /// - /// Called after an entity type member is ignored. - /// - /// The builder for the entity type. - /// The name of the ignored member. - /// Additional information associated with convention execution. + /// + public virtual void ProcessForeignKeyPrincipalEndChanged( + IConventionForeignKeyBuilder relationshipBuilder, + IConventionContext context) + { + var fk = relationshipBuilder.Metadata; + var dependentToPrincipalAttributes = fk.DependentToPrincipal == null + ? null + : GetAttributes(fk.DeclaringEntityType, fk.DependentToPrincipal); + var principalToDependentAttributes = fk.PrincipalToDependent == null + ? null + : GetAttributes(fk.PrincipalEntityType, fk.PrincipalToDependent); + ProcessForeignKeyPrincipalEndChanged( + relationshipBuilder, dependentToPrincipalAttributes, principalToDependentAttributes, context); + } + + /// public virtual void ProcessEntityTypeMemberIgnored( IConventionEntityTypeBuilder entityTypeBuilder, string name, IConventionContext context) { @@ -316,5 +328,19 @@ private Type FindCandidateNavigationWithAttributePropertyType([NotNull] Property [NotNull] TAttribute attribute, [NotNull] IConventionContext context) => throw new NotImplementedException(); + + /// + /// Called after the principal end of a foreign key is changed. + /// + /// The builder for the foreign key. + /// The attributes on the dependent to principal navigation. + /// The attributes on the principal to dependent navigation. + /// Additional information associated with convention execution. + public virtual void ProcessForeignKeyPrincipalEndChanged( + [NotNull] IConventionForeignKeyBuilder relationshipBuilder, + [CanBeNull] IEnumerable dependentToPrincipalAttributes, + [CanBeNull] IEnumerable principalToDependentAttributes, + [NotNull] IConventionContext context) + => throw new NotImplementedException(); } } diff --git a/src/EFCore/Metadata/Conventions/NonNullableNavigationConvention.cs b/src/EFCore/Metadata/Conventions/NonNullableNavigationConvention.cs index fcea0f4a019..2a411405ddd 100644 --- a/src/EFCore/Metadata/Conventions/NonNullableNavigationConvention.cs +++ b/src/EFCore/Metadata/Conventions/NonNullableNavigationConvention.cs @@ -14,7 +14,10 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Conventions /// /// A convention that configures the non-nullable navigations to principal entity type as required. /// - public class NonNullableNavigationConvention : NonNullableConventionBase, INavigationAddedConvention + public class NonNullableNavigationConvention : + NonNullableConventionBase, + INavigationAddedConvention, + IForeignKeyPrincipalEndChangedConvention { /// /// Creates a new instance of . @@ -29,6 +32,31 @@ public NonNullableNavigationConvention([NotNull] ProviderConventionSetBuilderDep public virtual void ProcessNavigationAdded( IConventionNavigationBuilder navigationBuilder, IConventionContext context) + { + ProcessNavigation(navigationBuilder); + context.StopProcessingIfChanged(navigationBuilder.Metadata.Builder); + } + + /// + public virtual void ProcessForeignKeyPrincipalEndChanged( + IConventionForeignKeyBuilder relationshipBuilder, + IConventionContext context) + { + var fk = relationshipBuilder.Metadata; + if (fk.DependentToPrincipal != null) + { + ProcessNavigation(fk.DependentToPrincipal.Builder); + } + + if (fk.PrincipalToDependent != null) + { + ProcessNavigation(fk.PrincipalToDependent.Builder); + } + + context.StopProcessingIfChanged(relationshipBuilder.Metadata.Builder); + } + + private void ProcessNavigation(IConventionNavigationBuilder navigationBuilder) { var navigation = navigationBuilder.Metadata; var foreignKey = navigation.ForeignKey; @@ -53,31 +81,16 @@ public NonNullableNavigationConvention([NotNull] ProviderConventionSetBuilderDep } } - if (!navigation.ForeignKey.IsUnique - || foreignKey.GetPrincipalEndConfigurationSource() != null) + if (foreignKey.GetPrincipalEndConfigurationSource() != null) { Dependencies.Logger.NonNullableReferenceOnDependent(navigation.ForeignKey.PrincipalToDependent); return; } - relationshipBuilder = relationshipBuilder.HasEntityTypes( - foreignKey.DeclaringEntityType, - foreignKey.PrincipalEntityType); - - if (relationshipBuilder == null) - { - return; - } - - Dependencies.Logger.NonNullableInverted(relationshipBuilder.Metadata.DependentToPrincipal); + return; } - relationshipBuilder = relationshipBuilder.IsRequired(true); - - if (relationshipBuilder != null) - { - context.StopProcessingIfChanged(relationshipBuilder.Metadata.DependentToPrincipal?.Builder); - } + relationshipBuilder.IsRequired(true); } private bool IsNonNullable(IConventionModelBuilder modelBuilder, IConventionNavigation navigation) diff --git a/src/EFCore/Metadata/Conventions/RequiredNavigationAttributeConvention.cs b/src/EFCore/Metadata/Conventions/RequiredNavigationAttributeConvention.cs index 741ceca745e..833d3866246 100644 --- a/src/EFCore/Metadata/Conventions/RequiredNavigationAttributeConvention.cs +++ b/src/EFCore/Metadata/Conventions/RequiredNavigationAttributeConvention.cs @@ -1,6 +1,7 @@ // 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.Collections.Generic; using System.ComponentModel.DataAnnotations; using System.Linq; using JetBrains.Annotations; @@ -30,6 +31,35 @@ public RequiredNavigationAttributeConvention([NotNull] ProviderConventionSetBuil IConventionNavigationBuilder navigationBuilder, RequiredAttribute attribute, IConventionContext context) + { + ProcessNavigation(navigationBuilder); + context.StopProcessingIfChanged(navigationBuilder.Metadata.Builder); + } + + /// + public override void ProcessForeignKeyPrincipalEndChanged( + IConventionForeignKeyBuilder relationshipBuilder, + IEnumerable dependentToPrincipalAttributes, + IEnumerable principalToDependentAttributes, + IConventionContext context) + { + var fk = relationshipBuilder.Metadata; + if (dependentToPrincipalAttributes != null + && dependentToPrincipalAttributes.Any()) + { + ProcessNavigation(fk.DependentToPrincipal.Builder); + } + + if (principalToDependentAttributes != null + && principalToDependentAttributes.Any()) + { + ProcessNavigation(fk.PrincipalToDependent.Builder); + } + + context.StopProcessingIfChanged(relationshipBuilder.Metadata.Builder); + } + + private void ProcessNavigation(IConventionNavigationBuilder navigationBuilder) { var navigation = navigationBuilder.Metadata; var foreignKey = navigation.ForeignKey; @@ -59,25 +89,10 @@ public RequiredNavigationAttributeConvention([NotNull] ProviderConventionSetBuil return; } - relationshipBuilder = relationshipBuilder.HasEntityTypes( - foreignKey.DeclaringEntityType, - foreignKey.PrincipalEntityType); - - if (relationshipBuilder == null) - { - return; - } - - Dependencies.Logger.RequiredAttributeInverted(relationshipBuilder.Metadata.DependentToPrincipal); - } - - relationshipBuilder = relationshipBuilder.IsRequired(true, fromDataAnnotation: true); - if (relationshipBuilder == null) - { return; } - context.StopProcessingIfChanged(relationshipBuilder.Metadata.DependentToPrincipal?.Builder); + relationshipBuilder.IsRequired(true, fromDataAnnotation: true); } } } diff --git a/src/EFCore/Properties/CoreStrings.Designer.cs b/src/EFCore/Properties/CoreStrings.Designer.cs index 080e322abde..aca0c1fa8bd 100644 --- a/src/EFCore/Properties/CoreStrings.Designer.cs +++ b/src/EFCore/Properties/CoreStrings.Designer.cs @@ -3894,54 +3894,6 @@ public static EventDefinition LogPossibleUnintendedCollectionNavigationN return (EventDefinition)definition; } - /// - /// The navigation property '{navigation}' has a RequiredAttribute causing the entity type '{entityType}' to be configured as the dependent side in the corresponding relationship. - /// - public static EventDefinition LogRequiredAttributeInverted([NotNull] IDiagnosticsLogger logger) - { - var definition = ((LoggingDefinitions)logger.Definitions).LogRequiredAttributeInverted; - if (definition == null) - { - definition = LazyInitializer.EnsureInitialized( - ref ((LoggingDefinitions)logger.Definitions).LogRequiredAttributeInverted, - () => new EventDefinition( - logger.Options, - CoreEventId.RequiredAttributeInverted, - LogLevel.Debug, - "CoreEventId.RequiredAttributeInverted", - level => LoggerMessage.Define( - level, - CoreEventId.RequiredAttributeInverted, - _resourceManager.GetString("LogRequiredAttributeInverted")))); - } - - return (EventDefinition)definition; - } - - /// - /// The navigation property '{navigation}' is non-nullable, causing the entity type '{entityType}' to be configured as the dependent side in the corresponding relationship. - /// - public static EventDefinition LogNonNullableInverted([NotNull] IDiagnosticsLogger logger) - { - var definition = ((LoggingDefinitions)logger.Definitions).LogNonNullableInverted; - if (definition == null) - { - definition = LazyInitializer.EnsureInitialized( - ref ((LoggingDefinitions)logger.Definitions).LogNonNullableInverted, - () => new EventDefinition( - logger.Options, - CoreEventId.NonNullableInverted, - LogLevel.Debug, - "CoreEventId.NonNullableInverted", - level => LoggerMessage.Define( - level, - CoreEventId.NonNullableInverted, - _resourceManager.GetString("LogNonNullableInverted")))); - } - - return (EventDefinition)definition; - } - /// /// The RequiredAttribute on '{principalEntityType}.{principalNavigation}' was ignored because there is also a RequiredAttribute on '{dependentEntityType}.{dependentNavigation}'. RequiredAttribute should only be specified on the dependent side of the relationship. /// @@ -4279,7 +4231,7 @@ public static EventDefinition LogOptimisticConcurrencyException([NotN } /// - /// The RequiredAttribute on '{principalEntityType}.{principalNavigation}' was ignored because it is pointing to the dependent entity. RequiredAttribute should only be specified on the navigation pointing to the principal side of the relationship. To change the dependent side configure the foreign key properties. + /// The RequiredAttribute on '{principalEntityType}.{principalNavigation}' is invalid. RequiredAttribute should only be specified on the navigation pointing to the principal side of the relationship. To change the dependent side configure the foreign key properties. /// public static EventDefinition LogRequiredAttributeOnDependent([NotNull] IDiagnosticsLogger logger) { @@ -4291,7 +4243,7 @@ public static EventDefinition LogOptimisticConcurrencyException([NotN () => new EventDefinition( logger.Options, CoreEventId.RequiredAttributeOnDependent, - LogLevel.Debug, + LogLevel.Error, "CoreEventId.RequiredAttributeOnDependent", level => LoggerMessage.Define( level, diff --git a/src/EFCore/Properties/CoreStrings.resx b/src/EFCore/Properties/CoreStrings.resx index b6f8e401e7c..764c5794da6 100644 --- a/src/EFCore/Properties/CoreStrings.resx +++ b/src/EFCore/Properties/CoreStrings.resx @@ -1015,14 +1015,6 @@ 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. Debug CoreEventId.IncompatibleMatchingForeignKeyProperties string string string string - - The navigation property '{navigation}' has a RequiredAttribute causing the entity type '{entityType}' to be configured as the dependent side in the corresponding relationship. - Debug CoreEventId.RequiredAttributeInverted string string - - - The navigation property '{navigation}' is non-nullable, causing the entity type '{entityType}' to be configured as the dependent side in the corresponding relationship. - Debug CoreEventId.NonNullableInverted string string - The RequiredAttribute on '{principalEntityType}.{principalNavigation}' was ignored because there is also a RequiredAttribute on '{dependentEntityType}.{dependentNavigation}'. RequiredAttribute should only be specified on the dependent side of the relationship. Debug CoreEventId.RequiredAttributeOnBothNavigations string string string string @@ -1221,8 +1213,8 @@ Debug CoreEventId.RequiredAttributeOnCollection string string - The RequiredAttribute on '{principalEntityType}.{principalNavigation}' was ignored because it is pointing to the dependent entity. RequiredAttribute should only be specified on the navigation pointing to the principal side of the relationship. To change the dependent side configure the foreign key properties. - Debug CoreEventId.RequiredAttributeOnDependent string string + The RequiredAttribute on '{principalEntityType}.{principalNavigation}' is invalid. RequiredAttribute should only be specified on the navigation pointing to the principal side of the relationship. To change the dependent side configure the foreign key properties. + Error CoreEventId.RequiredAttributeOnDependent string string '{principalEntityType}.{principalNavigation}' may still be null at runtime despite being declared as non-nullable since only the navigation to principal can be configured as required. diff --git a/test/EFCore.Tests/Metadata/Conventions/NavigationAttributeConventionTest.cs b/test/EFCore.Tests/Metadata/Conventions/NavigationAttributeConventionTest.cs index aff77d12826..72e590aab16 100644 --- a/test/EFCore.Tests/Metadata/Conventions/NavigationAttributeConventionTest.cs +++ b/test/EFCore.Tests/Metadata/Conventions/NavigationAttributeConventionTest.cs @@ -173,7 +173,7 @@ public void RequiredAttribute_does_not_set_is_required_for_collection_navigation } [ConditionalFact] - public void RequiredAttribute_does_not_set_is_required_for_navigation_to_dependent() + public void RequiredAttribute_throws_for_navigation_to_dependent() { var dependentEntityTypeBuilder = CreateInternalEntityTypeBuilder(); var principalEntityTypeBuilder = @@ -190,20 +190,16 @@ public void RequiredAttribute_does_not_set_is_required_for_navigation_to_depende var navigation = principalEntityTypeBuilder.Metadata.FindNavigation(nameof(Principal.Dependent)); Assert.False(relationshipBuilder.Metadata.IsRequired); - - RunRequiredNavigationAttributeConvention(relationshipBuilder, navigation); - - Assert.False(relationshipBuilder.Metadata.IsRequired); - - var logEntry = ListLoggerFactory.Log.Single(); - Assert.Equal(LogLevel.Debug, logEntry.Level); - Assert.Equal( - CoreResources.LogRequiredAttributeOnDependent(new TestLogger()).GenerateMessage( - nameof(Dependent), nameof(Dependent.Principal)), logEntry.Message); + + Assert.Equal(CoreStrings.WarningAsErrorTemplate( + CoreEventId.RequiredAttributeOnDependent.ToString(), + CoreResources.LogRequiredAttributeOnDependent(new TestLogger()) + .GenerateMessage(nameof(Dependent.Principal), nameof(Dependent)), CoreEventId.RequiredAttributeOnDependent), + Assert.Throws(() => RunRequiredNavigationAttributeConvention(relationshipBuilder, navigation)).Message); } [ConditionalFact] - public void RequiredAttribute_inverts_when_navigation_to_dependent() + public void RequiredAttribute_does_nothing_when_principal_end_is_ambiguous() { var dependentEntityTypeBuilder = CreateInternalEntityTypeBuilder(); var principalEntityTypeBuilder = @@ -222,16 +218,10 @@ public void RequiredAttribute_inverts_when_navigation_to_dependent() RunRequiredNavigationAttributeConvention(relationshipBuilder, navigation); - var newForeignKey = principalEntityTypeBuilder.Metadata.GetForeignKeys().Single(); - Assert.Equal(nameof(Principal.Dependent), newForeignKey.DependentToPrincipal.Name); - Assert.Equal(nameof(Principal), newForeignKey.DeclaringEntityType.DisplayName()); - Assert.True(newForeignKey.IsRequired); - - var logEntry = ListLoggerFactory.Log.Single(); - Assert.Equal(LogLevel.Debug, logEntry.Level); - Assert.Equal( - CoreResources.LogRequiredAttributeInverted(new TestLogger()).GenerateMessage( - nameof(Principal.Dependent), nameof(Principal)), logEntry.Message); + var newForeignKey = dependentEntityTypeBuilder.Metadata.GetForeignKeys().Single(); + Assert.Equal(nameof(Principal.Dependent), newForeignKey.PrincipalToDependent.Name); + Assert.False(newForeignKey.IsRequired); + Assert.Empty(ListLoggerFactory.Log); } [ConditionalFact] @@ -251,6 +241,7 @@ public void RequiredAttribute_can_be_specified_on_both_navigations() { var modelBuilder = CreateModelBuilder(); var model = (Model)modelBuilder.Model; + modelBuilder.Entity(); modelBuilder.Entity().HasOne(b => b.Blog).WithOne(b => b.BlogDetails); Assert.True( @@ -1040,7 +1031,7 @@ public void BackingFieldAttribute_does_not_override_configuration_from_explicit_ public void Navigation_attribute_convention_runs_for_private_property() { var modelBuilder = CreateModelBuilder(); - var referenceBuilder = modelBuilder.Entity().HasOne("Post").WithOne(); + var referenceBuilder = modelBuilder.Entity().HasOne("Post").WithOne().HasForeignKey(); Assert.False(referenceBuilder.Metadata.Properties.First().IsNullable); } diff --git a/test/EFCore.Tests/Metadata/Conventions/NonNullableNavigationConventionTest.cs b/test/EFCore.Tests/Metadata/Conventions/NonNullableNavigationConventionTest.cs index 9f5bdafc100..9b415f171c3 100644 --- a/test/EFCore.Tests/Metadata/Conventions/NonNullableNavigationConventionTest.cs +++ b/test/EFCore.Tests/Metadata/Conventions/NonNullableNavigationConventionTest.cs @@ -120,7 +120,7 @@ public void Non_nullability_does_not_set_is_required_for_navigation_to_dependent } [ConditionalFact] - public void Non_nullability_inverts_when_navigation_to_dependent() + public void Non_nullability_logs_when_navigation_to_dependent() { var dependentEntityTypeBuilder = CreateInternalEntityTypeBuilder(); var principalEntityTypeBuilder = @@ -139,14 +139,24 @@ public void Non_nullability_inverts_when_navigation_to_dependent() navigation = RunConvention(relationshipBuilder, navigation); - Assert.Equal(nameof(Principal), navigation.ForeignKey.DeclaringEntityType.DisplayName()); - Assert.True(navigation.ForeignKey.IsRequired); + Assert.Equal(nameof(Dependent), navigation.ForeignKey.DeclaringEntityType.DisplayName()); + Assert.False(navigation.ForeignKey.IsRequired); + Assert.Empty(ListLoggerFactory.Log); + + relationshipBuilder.HasEntityTypes( + relationshipBuilder.Metadata.PrincipalEntityType, + relationshipBuilder.Metadata.DeclaringEntityType, + ConfigurationSource.Convention); + + navigation = RunConvention(relationshipBuilder, navigation); + Assert.Equal(nameof(Dependent), navigation.ForeignKey.DeclaringEntityType.DisplayName()); + Assert.False(navigation.ForeignKey.IsRequired); var logEntry = ListLoggerFactory.Log.Single(); Assert.Equal(LogLevel.Debug, logEntry.Level); Assert.Equal( - CoreResources.LogNonNullableInverted(new TestLogger()).GenerateMessage( - nameof(Principal.Dependent), nameof(Principal)), logEntry.Message); + CoreResources.LogNonNullableReferenceOnDependent(new TestLogger()).GenerateMessage( + nameof(Dependent.Principal), nameof(Dependent)), logEntry.Message); } [ConditionalFact]