diff --git a/src/EFCore/Diagnostics/CoreEventId.cs b/src/EFCore/Diagnostics/CoreEventId.cs index f5bdb484c37..72461d02950 100644 --- a/src/EFCore/Diagnostics/CoreEventId.cs +++ b/src/EFCore/Diagnostics/CoreEventId.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; using System.ComponentModel.DataAnnotations; using System.ComponentModel.DataAnnotations.Schema; using System.Diagnostics; @@ -97,10 +98,10 @@ private enum Id ForeignKeyAttributesOnBothNavigationsWarning, ConflictingForeignKeyAttributesOnNavigationAndPropertyWarning, RedundantForeignKeyWarning, - NonNullableInverted, + NonNullableInverted, // Unused NonNullableReferenceOnBothNavigations, NonNullableReferenceOnDependent, - RequiredAttributeInverted, + RequiredAttributeInverted, // Unused RequiredAttributeOnCollection, CollectionWithoutComparer, ConflictingKeylessAndKeyAttributesWarning, @@ -405,6 +406,7 @@ public static readonly EventId InvalidIncludePathError /// This event uses the payload when used with a . /// /// + [Obsolete] public static readonly EventId RequiredAttributeInverted = MakeModelId(Id.RequiredAttributeInverted); /// @@ -419,6 +421,7 @@ public static readonly EventId InvalidIncludePathError /// This event uses the payload when used with a . /// /// + [Obsolete] public static readonly EventId NonNullableInverted = MakeModelId(Id.NonNullableInverted); /// diff --git a/src/EFCore/Diagnostics/CoreLoggerExtensions.cs b/src/EFCore/Diagnostics/CoreLoggerExtensions.cs index b7d3302c616..1bd17c3b16c 100644 --- a/src/EFCore/Diagnostics/CoreLoggerExtensions.cs +++ b/src/EFCore/Diagnostics/CoreLoggerExtensions.cs @@ -8,7 +8,6 @@ using System.Linq.Expressions; using System.Reflection; using JetBrains.Annotations; -using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.ChangeTracking; using Microsoft.EntityFrameworkCore.ChangeTracking.Internal; using Microsoft.EntityFrameworkCore.Diagnostics.Internal; @@ -1172,6 +1171,7 @@ private static string IncompatibleMatchingForeignKeyProperties(EventDefinitionBa /// /// The diagnostics logger to use. /// The navigation property. + [Obsolete] public static void RequiredAttributeInverted( [NotNull] this IDiagnosticsLogger diagnostics, [NotNull] INavigation navigation) @@ -1180,7 +1180,7 @@ public static void RequiredAttributeInverted( if (diagnostics.ShouldLog(definition)) { - definition.Log(diagnostics,navigation.Name, navigation.DeclaringEntityType.DisplayName()); + definition.Log(diagnostics, navigation.Name, navigation.DeclaringEntityType.DisplayName()); } if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled)) @@ -1206,6 +1206,7 @@ private static string RequiredAttributeInverted(EventDefinitionBase definition, /// /// The diagnostics logger to use. /// The navigation property. + [Obsolete] public static void NonNullableInverted( [NotNull] this IDiagnosticsLogger diagnostics, [NotNull] INavigation navigation) @@ -1214,7 +1215,7 @@ public static void NonNullableInverted( if (diagnostics.ShouldLog(definition)) { - definition.Log(diagnostics,navigation.Name, navigation.DeclaringEntityType.DisplayName()); + definition.Log(diagnostics, navigation.Name, navigation.DeclaringEntityType.DisplayName()); } if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled)) @@ -1346,7 +1347,7 @@ public static void RequiredAttributeOnDependent( { 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 +1365,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 +1383,8 @@ public static void NonNullableReferenceOnDependent( { 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 +1402,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..f28ed7d3b1f 100644 --- a/src/EFCore/Diagnostics/LoggingDefinitions.cs +++ b/src/EFCore/Diagnostics/LoggingDefinitions.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; using Microsoft.EntityFrameworkCore.Infrastructure; namespace Microsoft.EntityFrameworkCore.Diagnostics @@ -500,6 +501,7 @@ public abstract class LoggingDefinitions /// doing so can result in application failures when updating to a new Entity Framework Core release. /// [EntityFrameworkInternal] + [Obsolete] public EventDefinitionBase LogRequiredAttributeInverted; /// @@ -509,6 +511,7 @@ public abstract class LoggingDefinitions /// doing so can result in application failures when updating to a new Entity Framework Core release. /// [EntityFrameworkInternal] + [Obsolete] public EventDefinitionBase LogNonNullableInverted; /// 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 @@ public virtual void ProcessNavigationAdded( } } - /// - /// 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 @@ public virtual void ProcessEntityTypeMemberIgnored( [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 virtual void ProcessNavigationAdded( } } - 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 override void ProcessNavigationAdded( 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 override void ProcessNavigationAdded( 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..497a6e09dcd 100644 --- a/src/EFCore/Properties/CoreStrings.Designer.cs +++ b/src/EFCore/Properties/CoreStrings.Designer.cs @@ -3897,6 +3897,7 @@ public static EventDefinition LogIncompatibleMat /// /// The navigation property '{navigation}' has a RequiredAttribute causing the entity type '{entityType}' to be configured as the dependent side in the corresponding relationship. /// + [Obsolete] public static EventDefinition LogRequiredAttributeInverted([NotNull] IDiagnosticsLogger logger) { var definition = ((LoggingDefinitions)logger.Definitions).LogRequiredAttributeInverted; @@ -3921,6 +3922,7 @@ public static EventDefinition LogRequiredAttributeInverted([NotN /// /// The navigation property '{navigation}' is non-nullable, causing the entity type '{entityType}' to be configured as the dependent side in the corresponding relationship. /// + [Obsolete] public static EventDefinition LogNonNullableInverted([NotNull] IDiagnosticsLogger logger) { var definition = ((LoggingDefinitions)logger.Definitions).LogNonNullableInverted; @@ -4279,7 +4281,7 @@ public static EventDefinition LogRequiredAttributeOnCollection([ } /// - /// 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 +4293,7 @@ public static EventDefinition LogRequiredAttributeOnDependent([N () => 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..07c4b3b7d95 100644 --- a/src/EFCore/Properties/CoreStrings.resx +++ b/src/EFCore/Properties/CoreStrings.resx @@ -1017,11 +1017,11 @@ 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 + Obsolete 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 + Obsolete 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. @@ -1221,8 +1221,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.Specification.Tests/DataAnnotationTestBase.cs b/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs index 74cbdcf95ff..cfb8d37ae7a 100644 --- a/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs +++ b/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs @@ -2210,15 +2210,16 @@ public virtual void RequiredAttribute_for_navigation_throws_while_inserting_null } [ConditionalFact] - public virtual void RequiredAttribute_does_nothing_when_specified_on_nav_to_dependent_per_convention() + public virtual void RequiredAttribute_throws_when_specified_on_nav_to_dependent_per_convention() { var modelBuilder = CreateModelBuilder(); - modelBuilder.Entity(); - var relationship = modelBuilder.Model.FindEntityType(typeof(AdditionalBookDetails)) - .FindNavigation(nameof(AdditionalBookDetails.BookDetails)).ForeignKey; - Assert.Equal(typeof(AdditionalBookDetails), relationship.PrincipalEntityType.ClrType); - Assert.False(relationship.IsRequired); + Assert.Equal(CoreStrings.WarningAsErrorTemplate( + CoreEventId.RequiredAttributeOnDependent.ToString(), + CoreResources.LogRequiredAttributeOnDependent(new TestLogger()) + .GenerateMessage(nameof(AdditionalBookDetails), nameof(AdditionalBookDetails.BookDetails)), + "CoreEventId.RequiredAttributeOnDependent"), + Assert.Throws(() => modelBuilder.Entity()).Message); } [ConditionalFact] diff --git a/test/EFCore.Tests/Metadata/Conventions/NavigationAttributeConventionTest.cs b/test/EFCore.Tests/Metadata/Conventions/NavigationAttributeConventionTest.cs index aff77d12826..3d5845859ac 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(Principal), nameof(Principal.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]