diff --git a/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs b/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs index 92360b5b83d..8f7a2fc0ece 100644 --- a/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs +++ b/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs @@ -176,6 +176,7 @@ public virtual ConventionSet CreateConventionSet() conventionSet.ModelFinalizedConventions.Add(nonNullableReferencePropertyConvention); conventionSet.ModelFinalizedConventions.Add(nonNullableNavigationConvention); conventionSet.ModelFinalizedConventions.Add(new QueryFilterDefiningQueryRewritingConvention(Dependencies)); + conventionSet.ModelFinalizedConventions.Add(inversePropertyAttributeConvention); conventionSet.ModelFinalizedConventions.Add(new ValidatingConvention(Dependencies)); // Don't add any more conventions to ModelFinalizedConventions after ValidatingConvention diff --git a/src/EFCore/Metadata/Conventions/InversePropertyAttributeConvention.cs b/src/EFCore/Metadata/Conventions/InversePropertyAttributeConvention.cs index b4ec96596f9..f9950b1571f 100644 --- a/src/EFCore/Metadata/Conventions/InversePropertyAttributeConvention.cs +++ b/src/EFCore/Metadata/Conventions/InversePropertyAttributeConvention.cs @@ -119,6 +119,19 @@ public InversePropertyAttributeConvention([NotNull] ProviderConventionSetBuilder var ambiguousInverse = FindAmbiguousInverse( navigationMemberInfo, entityType, referencingNavigationsWithAttribute); + var baseType = targetEntityTypeBuilder.Metadata.BaseType; + while(ambiguousInverse == null + && baseType != null) + { + var navigationMap = GetInverseNavigations(baseType); + if (navigationMap != null + && navigationMap.TryGetValue(inverseNavigationPropertyInfo.Name, out var inverseTuple)) { + referencingNavigationsWithAttribute = inverseTuple.References; + ambiguousInverse = FindAmbiguousInverse(navigationMemberInfo, entityType, referencingNavigationsWithAttribute); + } + baseType = baseType.BaseType; + } + if (ambiguousInverse != null) { var existingInverse = targetEntityTypeBuilder.Metadata.FindNavigation(inverseNavigationPropertyInfo)?.FindInverse(); @@ -268,6 +281,12 @@ public InversePropertyAttributeConvention([NotNull] ProviderConventionSetBuilder attribute); if (newRelationship != relationshipBuilder) { + if (newRelationship == null) + { + context.StopProcessingIfChanged(null); + return; + } + var newNavigation = navigation.IsDependentToPrincipal() ? newRelationship.Metadata.DependentToPrincipal : newRelationship.Metadata.PrincipalToDependent; @@ -356,14 +375,32 @@ public virtual void ProcessModelFinalized(IConventionModelBuilder modelBuilder, continue; } - foreach (var inverseNavigation in inverseNavigations) + foreach (var inverseNavigation in inverseNavigations.Values) { - foreach (var referencingNavigationWithAttribute in inverseNavigation.Value) + foreach (var referencingNavigationWithAttribute in inverseNavigation.References) { var ambiguousInverse = FindAmbiguousInverse( referencingNavigationWithAttribute.Item1, referencingNavigationWithAttribute.Item2, - inverseNavigation.Value); + inverseNavigation.References); + + var baseType = entityType.BaseType; + while (ambiguousInverse == null + && baseType != null) + { + var navigationMap = GetInverseNavigations(baseType); + if (navigationMap != null + && navigationMap.TryGetValue(inverseNavigation.Navigation.Name, out var inverseTuple)) + { + var referencingNavigationsWithAttribute = inverseTuple.References; + ambiguousInverse = FindAmbiguousInverse( + referencingNavigationWithAttribute.Item1, + referencingNavigationWithAttribute.Item2, + referencingNavigationsWithAttribute); + } + baseType = baseType.BaseType; + } + if (ambiguousInverse != null) { Dependencies.Logger.MultipleInversePropertiesSameTargetWarning( @@ -373,13 +410,18 @@ public virtual void ProcessModelFinalized(IConventionModelBuilder modelBuilder, referencingNavigationWithAttribute.Item1, referencingNavigationWithAttribute.Item2.ClrType), Tuple.Create(ambiguousInverse.Value.Item1, ambiguousInverse.Value.Item2.ClrType) }, - inverseNavigation.Key, + inverseNavigation.Navigation, entityType.ClrType); break; } } } } + + foreach (var entityType in model.GetEntityTypes()) + { + entityType.RemoveAnnotation(CoreAnnotationNames.InverseNavigations); + } } /// @@ -395,19 +437,26 @@ public virtual void ProcessModelFinalized(IConventionModelBuilder modelBuilder, public static bool IsAmbiguous( [NotNull] IConventionEntityType entityType, [NotNull] MemberInfo navigation, [NotNull] IConventionEntityType targetEntityType) { - var inverseNavigations = GetInverseNavigations(targetEntityType); - if (inverseNavigations == null) + if (!Attribute.IsDefined(navigation, typeof(InversePropertyAttribute))) { return false; } - foreach (var inverseNavigation in inverseNavigations) + while (targetEntityType != null) { - if (inverseNavigation.Key.GetMemberType().IsAssignableFrom(entityType.ClrType) - && IsAmbiguousInverse(navigation, entityType, inverseNavigation.Value)) + var navigationMap = GetInverseNavigations(targetEntityType); + if (navigationMap != null) { - return true; + foreach (var inverseNavigationTuple in navigationMap.Values) + { + if (inverseNavigationTuple.Navigation.GetMemberType().IsAssignableFrom(entityType.ClrType) + && IsAmbiguousInverse(navigation, entityType, inverseNavigationTuple.References)) + { + return true; + } + } } + targetEntityType = targetEntityType.BaseType; } return false; @@ -424,11 +473,6 @@ public virtual void ProcessModelFinalized(IConventionModelBuilder modelBuilder, IConventionEntityType entityType, List<(MemberInfo, IConventionEntityType)> referencingNavigationsWithAttribute) { - if (referencingNavigationsWithAttribute.Count == 1) - { - return null; - } - List<(MemberInfo, IConventionEntityType)> tuplesToRemove = null; (MemberInfo, IConventionEntityType)? ambiguousTuple = null; foreach (var referencingTuple in referencingNavigationsWithAttribute) @@ -471,14 +515,19 @@ public virtual void ProcessModelFinalized(IConventionModelBuilder modelBuilder, var inverseNavigations = GetInverseNavigations(targetEntityType); if (inverseNavigations == null) { - inverseNavigations = new Dictionary>(); + inverseNavigations = new Dictionary)>(); SetInverseNavigations(targetEntityType.Builder, inverseNavigations); } - if (!inverseNavigations.TryGetValue(inverseNavigation, out var referencingNavigationsWithAttribute)) + List<(MemberInfo, IConventionEntityType)> referencingNavigationsWithAttribute; + if (!inverseNavigations.TryGetValue(inverseNavigation.Name, out var inverseTuple)) { referencingNavigationsWithAttribute = new List<(MemberInfo, IConventionEntityType)>(); - inverseNavigations[inverseNavigation] = referencingNavigationsWithAttribute; + inverseNavigations[inverseNavigation.Name] = (inverseNavigation, referencingNavigationsWithAttribute); + } + else + { + referencingNavigationsWithAttribute = inverseTuple.References; } foreach (var referencingTuple in referencingNavigationsWithAttribute) @@ -508,11 +557,9 @@ public virtual void ProcessModelFinalized(IConventionModelBuilder modelBuilder, return; } - foreach (var inverseNavigationPair in inverseNavigations) + foreach (var inverseNavigationPair in inverseNavigations.Values) { - var inverseNavigation = inverseNavigationPair.Key; - var referencingNavigationsWithAttribute = inverseNavigationPair.Value; - + var (inverseNavigation, referencingNavigationsWithAttribute) = inverseNavigationPair; for (var index = 0; index < referencingNavigationsWithAttribute.Count; index++) { var referencingTuple = referencingNavigationsWithAttribute[index]; @@ -523,7 +570,7 @@ public virtual void ProcessModelFinalized(IConventionModelBuilder modelBuilder, referencingNavigationsWithAttribute.RemoveAt(index); if (referencingNavigationsWithAttribute.Count == 0) { - inverseNavigations.Remove(inverseNavigation); + inverseNavigations.Remove(inverseNavigation.Name); } if (referencingNavigationsWithAttribute.Count == 1) @@ -533,8 +580,8 @@ public virtual void ProcessModelFinalized(IConventionModelBuilder modelBuilder, { targetEntityType.Builder.HasRelationship( otherEntityType, - (PropertyInfo)inverseNavigation, - (PropertyInfo)referencingNavigationsWithAttribute[0].Item1, + inverseNavigation, + referencingNavigationsWithAttribute[0].Item1, fromDataAnnotation: true); } } @@ -548,14 +595,14 @@ public virtual void ProcessModelFinalized(IConventionModelBuilder modelBuilder, private static IConventionEntityType FindActualEntityType(IConventionEntityType entityType) => ((Model)entityType.Model).FindActualEntityType((EntityType)entityType); - private static Dictionary> GetInverseNavigations( + private static Dictionary References)> GetInverseNavigations( IConventionAnnotatable entityType) => entityType.FindAnnotation(CoreAnnotationNames.InverseNavigations)?.Value - as Dictionary>; + as Dictionary)>; private static void SetInverseNavigations( IConventionAnnotatableBuilder entityTypeBuilder, - Dictionary> inverseNavigations) + Dictionary)> inverseNavigations) => entityTypeBuilder.HasAnnotation(CoreAnnotationNames.InverseNavigations, inverseNavigations); } } diff --git a/src/EFCore/Metadata/Conventions/ModelCleanupConvention.cs b/src/EFCore/Metadata/Conventions/ModelCleanupConvention.cs index 064f1950579..620721ff510 100644 --- a/src/EFCore/Metadata/Conventions/ModelCleanupConvention.cs +++ b/src/EFCore/Metadata/Conventions/ModelCleanupConvention.cs @@ -92,7 +92,6 @@ private void RemoveModelBuildingAnnotations(IConventionModelBuilder modelBuilder { entityType.RemoveAnnotation(CoreAnnotationNames.AmbiguousNavigations); entityType.RemoveAnnotation(CoreAnnotationNames.NavigationCandidates); - entityType.RemoveAnnotation(CoreAnnotationNames.InverseNavigations); } } } diff --git a/src/EFCore/Properties/CoreStrings.Designer.cs b/src/EFCore/Properties/CoreStrings.Designer.cs index 126b1469076..5dba85f2e26 100644 --- a/src/EFCore/Properties/CoreStrings.Designer.cs +++ b/src/EFCore/Properties/CoreStrings.Designer.cs @@ -1369,7 +1369,7 @@ public static string ConflictingRelationshipNavigation([CanBeNull] object newPri newPrincipalEntityType, newPrincipalNavigation, newDependentEntityType, newDependentNavigation, existingPrincipalEntityType, existingPrincipalNavigation, existingDependentEntityType, existingDependentNavigation); /// - /// Error generated for warning '{eventName}: {message}'. This exception can be suppressed or logged by passing event ID '{eventId}' to the 'ConfigureWarnings' method in 'DbContext.OnConfiguring' or 'AddDbContext'. + /// Error generated for warning '{eventName}': {message} This exception can be suppressed or logged by passing event ID '{eventId}' to the 'ConfigureWarnings' method in 'DbContext.OnConfiguring' or 'AddDbContext'. /// public static string WarningAsErrorTemplate([CanBeNull] object eventName, [CanBeNull] object message, [CanBeNull] object eventId) => string.Format( diff --git a/src/EFCore/Properties/CoreStrings.resx b/src/EFCore/Properties/CoreStrings.resx index b10482078ae..c3b86d2eb36 100644 --- a/src/EFCore/Properties/CoreStrings.resx +++ b/src/EFCore/Properties/CoreStrings.resx @@ -782,7 +782,7 @@ Cannot create a relationship between '{newPrincipalEntityType}.{newPrincipalNavigation}' and '{newDependentEntityType}.{newDependentNavigation}', because there already is a relationship between '{existingPrincipalEntityType}.{existingPrincipalNavigation}' and '{existingDependentEntityType}.{existingDependentNavigation}'. Navigation properties can only participate in a single relationship. - Error generated for warning '{eventName}: {message}'. This exception can be suppressed or logged by passing event ID '{eventId}' to the 'ConfigureWarnings' method in 'DbContext.OnConfiguring' or 'AddDbContext'. + Error generated for warning '{eventName}': {message} This exception can be suppressed or logged by passing event ID '{eventId}' to the 'ConfigureWarnings' method in 'DbContext.OnConfiguring' or 'AddDbContext'. Cannot access a disposed object. A common cause of this error is disposing a context that was resolved from dependency injection and then later trying to use the same context instance elsewhere in your application. This may occur if you are calling Dispose() on the context, or wrapping the context in a using statement. If you are using dependency injection, you should let the dependency injection container take care of disposing context instances. diff --git a/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs b/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs index 149cd654955..2af74b734f9 100644 --- a/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs +++ b/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs @@ -1318,12 +1318,12 @@ public virtual void ForeignKeyAttribute_configures_relationships_when_inverse_on Assert.Equal(nameof(PartialAnswerRepeating.AnswerId), fk1.Properties.Single().Name); } - public abstract class Answer + private abstract class Answer { public int Id { get; set; } } - public class PartialAnswerBase + private class PartialAnswerBase { public int Id { get; set; } public int AnswerId { get; set; } @@ -1332,20 +1332,20 @@ public class PartialAnswerBase public virtual Answer Answer { get; set; } } - public class PartialAnswer : PartialAnswerBase + private class PartialAnswer : PartialAnswerBase { } - public class PartialAnswerRepeating : PartialAnswerBase + private class PartialAnswerRepeating : PartialAnswerBase { } - public class MultipleAnswers : Answer + private class MultipleAnswers : Answer { public virtual ICollection Answers { get; set; } } - public class MultipleAnswersRepeating : Answer + private class MultipleAnswersRepeating : Answer { public virtual ICollection Answers { get; set; } } @@ -1841,6 +1841,51 @@ protected class SpecialPost7698 : Post7698 public Blog7698 BlogInverseNav { get; set; } } + [ConditionalFact] + public virtual void InversePropertyAttribute_pointing_to_same_nav_on_base_causes_ambiguity() + { + var modelBuilder = CreateModelBuilder(); + modelBuilder.Entity(); + modelBuilder.Entity(); + + Assert.Equal( + CoreStrings.WarningAsErrorTemplate( + CoreEventId.MultipleInversePropertiesSameTargetWarning, + CoreResources.LogMultipleInversePropertiesSameTarget(new TestLogger()) + .GenerateMessage( + $"{nameof(MultipleAnswersRepeatingInverse)}.{nameof(MultipleAnswersRepeatingInverse.Answers)}," + + $" {nameof(MultipleAnswersInverse)}.{nameof(MultipleAnswersInverse.Answers)}", + nameof(PartialAnswerInverse.Answer)), + "CoreEventId.MultipleInversePropertiesSameTargetWarning"), + Assert.Throws(() => modelBuilder.FinalizeModel()).Message); + } + + private class PartialAnswerInverse + { + public int Id { get; set; } + public int AnswerId { get; set; } + public virtual AnswerBaseInverse Answer { get; set; } + } + + private class PartialAnswerRepeatingInverse : PartialAnswerInverse { } + + private abstract class AnswerBaseInverse + { + public int Id { get; set; } + } + + private class MultipleAnswersInverse : AnswerBaseInverse + { + [InverseProperty("Answer")] + public virtual ICollection Answers { get; set; } + } + + private class MultipleAnswersRepeatingInverse : AnswerBaseInverse + { + [InverseProperty("Answer")] + public virtual IEnumerable Answers { get; set; } + } + [ConditionalFact] public virtual void ForeignKeyAttribute_creates_two_relationships_if_applied_on_property_on_both_side() {