From 8d9988c412d947ab225bcffcf600e3aabc7515c5 Mon Sep 17 00:00:00 2001 From: Andriy Svyryd Date: Wed, 29 Jul 2020 17:45:45 -0700 Subject: [PATCH] Add IConventionEntityTypeBuilder.HasSkipNavigation that takes an inverse Fixes #21486 --- .../Builders/IConventionEntityTypeBuilder.cs | 34 ++++- .../RelationshipDiscoveryConvention.cs | 49 +------ src/EFCore/Metadata/Internal/EntityType.cs | 8 ++ .../Internal/InternalEntityTypeBuilder.cs | 121 +++++++++++++----- .../Metadata/Internal/SkipNavigation.cs | 16 ++- 5 files changed, 144 insertions(+), 84 deletions(-) diff --git a/src/EFCore/Metadata/Builders/IConventionEntityTypeBuilder.cs b/src/EFCore/Metadata/Builders/IConventionEntityTypeBuilder.cs index 920b07c2cb1..11deddebfc3 100644 --- a/src/EFCore/Metadata/Builders/IConventionEntityTypeBuilder.cs +++ b/src/EFCore/Metadata/Builders/IConventionEntityTypeBuilder.cs @@ -619,6 +619,32 @@ bool CanAddNavigation([NotNull] string navigationName, bool fromDataAnnotation = /// if the configuration can be applied. bool CanHaveNavigation([NotNull] string navigationName, bool fromDataAnnotation = false); + /// + /// Configures a skip navigation and the inverse between this and the target entity type. + /// + /// The entity type that this relationship targets. + /// The navigation property on this entity type that is part of the relationship. + /// + /// The navigation property on the target entity type that is part of the relationship. If + /// is specified, the relationship will be configured without a navigation property on the target end. + /// + /// Whether both of the navigation properties are collections or aren't collections. + /// + /// Whether both of the navigation property are defined on the dependent side of the underlying foreign keys. + /// + /// Indicates whether the configuration was specified using a data annotation. + /// + /// An object that can be used to configure the relationship if it exists on the entity type, + /// otherwise. + /// + IConventionSkipNavigationBuilder HasSkipNavigation( + [NotNull] MemberInfo navigationToTarget, + [NotNull] IConventionEntityType targetEntityType, + [NotNull] MemberInfo inverseNavigation, + bool? collections = null, + bool? onDependent = null, + bool fromDataAnnotation = false); + /// /// Configures a skip navigation between this and the target entity type. /// @@ -636,8 +662,8 @@ bool CanAddNavigation([NotNull] string navigationName, bool fromDataAnnotation = IConventionSkipNavigationBuilder HasSkipNavigation( [NotNull] MemberInfo navigationToTarget, [NotNull] IConventionEntityType targetEntityType, - bool collection = true, - bool onDependent = false, + bool? collection = null, + bool? onDependent = null, bool fromDataAnnotation = false); /// @@ -657,8 +683,8 @@ IConventionSkipNavigationBuilder HasSkipNavigation( IConventionSkipNavigationBuilder HasSkipNavigation( [NotNull] string navigationName, [NotNull] IConventionEntityType targetEntityType, - bool collection = true, - bool onDependent = false, + bool? collection = null, + bool? onDependent = null, bool fromDataAnnotation = false); /// diff --git a/src/EFCore/Metadata/Conventions/RelationshipDiscoveryConvention.cs b/src/EFCore/Metadata/Conventions/RelationshipDiscoveryConvention.cs index dad6209e9a6..dec053e1ac2 100644 --- a/src/EFCore/Metadata/Conventions/RelationshipDiscoveryConvention.cs +++ b/src/EFCore/Metadata/Conventions/RelationshipDiscoveryConvention.cs @@ -664,12 +664,15 @@ private void CreateRelationships( { entityTypeBuilder.HasOwnership(targetEntityType.ClrType, navigation, inverse); } - else + else if (entityTypeBuilder.HasRelationship(targetEntityType, navigation, inverse) == null) { - if (entityTypeBuilder.HasRelationship(targetEntityType, navigation, inverse) == null) + var navigationTargetType = navigation.PropertyType.TryGetSequenceType(); + var inverseTargetType = inverse.PropertyType.TryGetSequenceType(); + if (navigationTargetType == targetEntityType.ClrType + && inverseTargetType == entityType.ClrType) { - HasManyToManyRelationship( - entityTypeBuilder, relationshipCandidate.TargetTypeBuilder, navigation, inverse); + entityTypeBuilder.HasSkipNavigation( + navigation, targetEntityType, inverse, collections: true, onDependent: false); } } } @@ -750,44 +753,6 @@ private void RemoveNavigation( } } - private void HasManyToManyRelationship( - IConventionEntityTypeBuilder entityTypeBuilder, - IConventionEntityTypeBuilder targetEntityTypeBuilder, - PropertyInfo navigation, - PropertyInfo inverse) - { - var navigationTargetType = navigation.PropertyType.TryGetSequenceType(); - var inverseTargetType = inverse.PropertyType.TryGetSequenceType(); - if (navigationTargetType == null - || inverseTargetType == null) - { - return; - } - - var entityType = entityTypeBuilder.Metadata; - var targetEntityType = targetEntityTypeBuilder.Metadata; - if (navigationTargetType != targetEntityType.ClrType - || inverseTargetType != entityType.ClrType) - { - return; - } - - var skipNavigationBuilder = entityTypeBuilder.HasSkipNavigation(navigation, targetEntityType); - if (skipNavigationBuilder == null) - { - return; - } - - var inverseSkipNavigationBuilder = targetEntityTypeBuilder.HasSkipNavigation(inverse, entityType); - if (inverseSkipNavigationBuilder == null) - { - entityTypeBuilder.HasNoSkipNavigation(skipNavigationBuilder.Metadata); - return; - } - - skipNavigationBuilder.HasInverse(inverseSkipNavigationBuilder.Metadata, fromDataAnnotation: false); - } - /// public virtual void ProcessEntityTypeAdded( IConventionEntityTypeBuilder entityTypeBuilder, IConventionContext context) diff --git a/src/EFCore/Metadata/Internal/EntityType.cs b/src/EFCore/Metadata/Internal/EntityType.cs index f922034117e..e3222f096fc 100644 --- a/src/EFCore/Metadata/Internal/EntityType.cs +++ b/src/EFCore/Metadata/Internal/EntityType.cs @@ -1663,6 +1663,14 @@ public virtual SkipNavigation AddSkipNavigation( shouldThrow: true); } + Navigation.IsCompatible( + name, + memberInfo, + this, + targetEntityType, + collection, + shouldThrow: true); + var skipNavigation = new SkipNavigation( name, memberInfo as PropertyInfo, diff --git a/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs b/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs index 29f9c93645e..8740e07ab97 100644 --- a/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs +++ b/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs @@ -3519,6 +3519,38 @@ private ForeignKey SetOrAddForeignKey( return foreignKey; } + /// + /// 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. + /// + public virtual InternalSkipNavigationBuilder HasSkipNavigation( + MemberIdentity navigationToTarget, + [NotNull] EntityType targetEntityType, + MemberIdentity inverseNavigation, + bool? collections, + bool? onDependent, + ConfigurationSource configurationSource) + { + var skipNavigationBuilder = HasSkipNavigation( + navigationToTarget, targetEntityType, configurationSource, collections, onDependent); + if (skipNavigationBuilder == null) + { + return null; + } + + var inverseSkipNavigationBuilder = targetEntityType.Builder.HasSkipNavigation( + inverseNavigation, Metadata, configurationSource, collections, onDependent); + if (inverseSkipNavigationBuilder == null) + { + HasNoSkipNavigation(skipNavigationBuilder.Metadata, configurationSource); + return null; + } + + return skipNavigationBuilder.HasInverse(inverseSkipNavigationBuilder.Metadata, configurationSource); + } + /// /// 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 @@ -3529,8 +3561,8 @@ public virtual InternalSkipNavigationBuilder HasSkipNavigation( MemberIdentity navigationProperty, [NotNull] EntityType targetEntityType, ConfigurationSource? configurationSource, - bool collection = true, - bool onDependent = false) + bool? collection = null, + bool? onDependent = null) { List navigationsToDetach = null; List<(InternalSkipNavigationBuilder Navigation, InternalSkipNavigationBuilder Inverse)> detachedNavigations = null; @@ -3542,11 +3574,11 @@ public virtual InternalSkipNavigationBuilder HasSkipNavigation( Check.DebugAssert(memberInfo == null || memberInfo.IsSameAs(existingNavigation.GetIdentifyingMemberInfo()), "Expected memberInfo to be the same on the existing navigation"); - Check.DebugAssert(collection == existingNavigation.IsCollection, - "Only collection skip navigations are currently supported"); + Check.DebugAssert(collection == null || collection == existingNavigation.IsCollection, + "Expected existing navigation to have the same cardinality"); - Check.DebugAssert(onDependent == existingNavigation.IsOnDependent, - "Only skip navigations on principals are currently supported"); + Check.DebugAssert(onDependent == null || onDependent == existingNavigation.IsOnDependent, + "Expected existing navigation to be on the same side"); if (existingNavigation.DeclaringEntityType != Metadata) { @@ -3563,39 +3595,47 @@ public virtual InternalSkipNavigationBuilder HasSkipNavigation( return existingNavigation.Builder; } - else + + if (!configurationSource.HasValue + || IsIgnored(navigationName, configurationSource)) { - if (!configurationSource.HasValue - || IsIgnored(navigationName, configurationSource)) + return null; + } + + foreach (var conflictingMember in Metadata.FindPropertiesInHierarchy(navigationName).Cast() + .Concat(Metadata.FindNavigationsInHierarchy(navigationName)) + .Concat(Metadata.FindServicePropertiesInHierarchy(navigationName))) + { + if (!configurationSource.Overrides(conflictingMember.GetConfigurationSource())) { return null; } + } - foreach (var conflictingMember in Metadata.FindPropertiesInHierarchy(navigationName).Cast() - .Concat(Metadata.FindNavigationsInHierarchy(navigationName)) - .Concat(Metadata.FindServicePropertiesInHierarchy(navigationName))) + foreach (var derivedType in Metadata.GetDerivedTypes()) + { + var conflictingNavigation = derivedType.FindDeclaredSkipNavigation(navigationName); + if (conflictingNavigation != null) { - if (!configurationSource.Overrides(conflictingMember.GetConfigurationSource())) + if (navigationsToDetach == null) { - return null; + navigationsToDetach = new List(); } - } - - foreach (var derivedType in Metadata.GetDerivedTypes()) - { - var conflictingNavigation = derivedType.FindDeclaredSkipNavigation(navigationName); - if (conflictingNavigation != null) - { - if (navigationsToDetach == null) - { - navigationsToDetach = new List(); - } - navigationsToDetach.Add(conflictingNavigation); - } + navigationsToDetach.Add(conflictingNavigation); } } + if (collection == null + && navigationProperty.MemberInfo != null) + { + var navigationType = navigationProperty.MemberInfo.GetMemberType(); + var navigationTargetClrType = navigationType.TryGetSequenceType(); + collection = navigationTargetClrType != null + && navigationType != targetEntityType.ClrType + && navigationTargetClrType.IsAssignableFrom(targetEntityType.ClrType); + } + InternalSkipNavigationBuilder builder; using (ModelBuilder.Metadata.ConventionDispatcher.DelayConventions()) { @@ -3650,7 +3690,7 @@ public virtual InternalSkipNavigationBuilder HasSkipNavigation( builder = Metadata.AddSkipNavigation( navigationName, navigationProperty.MemberInfo, - targetEntityType, collection, onDependent, configurationSource.Value).Builder; + targetEntityType, collection ?? true, onDependent ?? false, configurationSource.Value).Builder; if (detachedNavigations != null) { @@ -4703,6 +4743,23 @@ IConventionForeignKeyBuilder IConventionEntityTypeBuilder.HasRelationship( fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention, setTargetAsPrincipal); + /// + [DebuggerStepThrough] + IConventionSkipNavigationBuilder IConventionEntityTypeBuilder.HasSkipNavigation( + MemberInfo navigationToTarget, + IConventionEntityType targetEntityType, + MemberInfo inverseNavigation, + bool? collections, + bool? onDependent, + bool fromDataAnnotation) + => HasSkipNavigation( + MemberIdentity.Create(navigationToTarget), + (EntityType)targetEntityType, + MemberIdentity.Create(inverseNavigation), + collections, + onDependent, + fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + /// /// 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 @@ -4819,8 +4876,8 @@ bool IConventionEntityTypeBuilder.CanHaveNavigation(string navigationName, bool IConventionSkipNavigationBuilder IConventionEntityTypeBuilder.HasSkipNavigation( MemberInfo navigationToTarget, IConventionEntityType targetEntityType, - bool collection, - bool onDependent, + bool? collection, + bool? onDependent, bool fromDataAnnotation) => HasSkipNavigation( MemberIdentity.Create(navigationToTarget), @@ -4834,8 +4891,8 @@ IConventionSkipNavigationBuilder IConventionEntityTypeBuilder.HasSkipNavigation( IConventionSkipNavigationBuilder IConventionEntityTypeBuilder.HasSkipNavigation( string navigationName, IConventionEntityType targetEntityType, - bool collection, - bool onDependent, + bool? collection, + bool? onDependent, bool fromDataAnnotation) => HasSkipNavigation( MemberIdentity.Create(navigationName), diff --git a/src/EFCore/Metadata/Internal/SkipNavigation.cs b/src/EFCore/Metadata/Internal/SkipNavigation.cs index c1d73e64b8f..426c188d425 100644 --- a/src/EFCore/Metadata/Internal/SkipNavigation.cs +++ b/src/EFCore/Metadata/Internal/SkipNavigation.cs @@ -336,60 +336,64 @@ public virtual DebugView DebugView [DebuggerStepThrough] public override string ToString() => this.ToDebugString(MetadataDebugStringOptions.SingleLineDefault); + /// IConventionSkipNavigationBuilder IConventionSkipNavigation.Builder { [DebuggerStepThrough] get => Builder; } - /// - /// 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. - /// + /// IConventionAnnotatableBuilder IConventionAnnotatable.Builder { [DebuggerStepThrough] get => Builder; } + /// IEntityType INavigationBase.DeclaringEntityType { [DebuggerStepThrough] get => DeclaringEntityType; } + /// IEntityType INavigationBase.TargetEntityType { [DebuggerStepThrough] get => TargetEntityType; } + /// IForeignKey ISkipNavigation.ForeignKey { [DebuggerStepThrough] get => ForeignKey; } + /// [DebuggerStepThrough] void IMutableSkipNavigation.SetForeignKey([CanBeNull] IMutableForeignKey foreignKey) => SetForeignKey((ForeignKey)foreignKey, ConfigurationSource.Explicit); + /// [DebuggerStepThrough] IConventionForeignKey IConventionSkipNavigation.SetForeignKey([CanBeNull] IConventionForeignKey foreignKey, bool fromDataAnnotation) => SetForeignKey((ForeignKey)foreignKey, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + /// ISkipNavigation ISkipNavigation.Inverse { [DebuggerStepThrough] get => Inverse; } + /// [DebuggerStepThrough] IMutableSkipNavigation IMutableSkipNavigation.SetInverse([CanBeNull] IMutableSkipNavigation inverse) => SetInverse((SkipNavigation)inverse, ConfigurationSource.Explicit); + /// [DebuggerStepThrough] IConventionSkipNavigation IConventionSkipNavigation.SetInverse([CanBeNull] IConventionSkipNavigation inverse, bool fromDataAnnotation) => SetInverse((SkipNavigation)inverse, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention);