diff --git a/src/EFCore/Metadata/Builders/IConventionEntityTypeBuilder.cs b/src/EFCore/Metadata/Builders/IConventionEntityTypeBuilder.cs index d5d6be71940..19c6cac3c73 100644 --- a/src/EFCore/Metadata/Builders/IConventionEntityTypeBuilder.cs +++ b/src/EFCore/Metadata/Builders/IConventionEntityTypeBuilder.cs @@ -619,6 +619,14 @@ bool CanAddNavigation([NotNull] string navigationName, bool fromDataAnnotation = /// if the configuration can be applied. bool CanHaveNavigation([NotNull] string navigationName, bool fromDataAnnotation = false); + /// + /// Returns a value indicating whether the given skinavigation can be added to this entity type. + /// + /// The name of the skip navigation. + /// Indicates whether the configuration was specified using a data annotation. + /// if the configuration can be applied. + bool CanHaveSkipNavigation([NotNull] string skipNavigationName, bool fromDataAnnotation = false); + /// /// Configures a skip navigation and the inverse between this and the target entity type. /// diff --git a/src/EFCore/Metadata/Conventions/InversePropertyAttributeConvention.cs b/src/EFCore/Metadata/Conventions/InversePropertyAttributeConvention.cs index a417285dca5..afc8233673f 100644 --- a/src/EFCore/Metadata/Conventions/InversePropertyAttributeConvention.cs +++ b/src/EFCore/Metadata/Conventions/InversePropertyAttributeConvention.cs @@ -53,8 +53,11 @@ private void Process( IConventionEntityTypeBuilder entityTypeBuilder, MemberInfo navigationMemberInfo, Type targetClrType, InversePropertyAttribute attribute) { - if (!entityTypeBuilder.CanHaveNavigation( - navigationMemberInfo.GetSimpleMemberName(), fromDataAnnotation: true)) + var canBeNavigation = entityTypeBuilder.CanHaveNavigation( + navigationMemberInfo.GetSimpleMemberName(), fromDataAnnotation: true); + var canBeSkipNavigation = entityTypeBuilder.CanHaveSkipNavigation( + navigationMemberInfo.GetSimpleMemberName(), fromDataAnnotation: true); + if (!canBeNavigation && !canBeSkipNavigation) { return; } @@ -67,14 +70,17 @@ private void Process( return; } - ConfigureInverseNavigation(entityTypeBuilder, navigationMemberInfo, targetEntityTypeBuilder, attribute); + ConfigureInverseNavigation( + entityTypeBuilder, navigationMemberInfo, targetEntityTypeBuilder, attribute, canBeNavigation, canBeSkipNavigation); } private IConventionForeignKeyBuilder ConfigureInverseNavigation( IConventionEntityTypeBuilder entityTypeBuilder, MemberInfo navigationMemberInfo, IConventionEntityTypeBuilder targetEntityTypeBuilder, - InversePropertyAttribute attribute) + InversePropertyAttribute attribute, + bool canBeNavigation, + bool canBeSkipNavigation) { var entityType = entityTypeBuilder.Metadata; var targetClrType = targetEntityTypeBuilder.Metadata.ClrType; @@ -137,53 +143,61 @@ private IConventionForeignKeyBuilder ConfigureInverseNavigation( if (ambiguousInverse != null) { - var existingInverse = targetEntityTypeBuilder.Metadata.FindNavigation(inverseNavigationPropertyInfo)?.Inverse; - var existingInverseType = existingInverse?.DeclaringEntityType; - if (existingInverse != null - && IsAmbiguousInverse( - existingInverse.GetIdentifyingMemberInfo(), existingInverseType, referencingNavigationsWithAttribute)) + if (canBeNavigation) { - var fk = existingInverse.ForeignKey; - if (fk.IsOwnership - || fk.DeclaringEntityType.Builder.HasNoRelationship(fk, fromDataAnnotation: true) == null) + var existingInverse = targetEntityTypeBuilder.Metadata.FindNavigation(inverseNavigationPropertyInfo)?.Inverse; + var existingInverseType = existingInverse?.DeclaringEntityType; + if (existingInverse != null + && IsAmbiguousInverse( + existingInverse.GetIdentifyingMemberInfo(), existingInverseType, referencingNavigationsWithAttribute)) { - fk.Builder.HasNavigation( - (string)null, - existingInverse.IsOnDependent, - fromDataAnnotation: true); + var fk = existingInverse.ForeignKey; + if (fk.IsOwnership + || fk.DeclaringEntityType.Builder.HasNoRelationship(fk, fromDataAnnotation: true) == null) + { + fk.Builder.HasNavigation( + (string)null, + existingInverse.IsOnDependent, + fromDataAnnotation: true); + } } - } - var existingNavigation = entityType.FindNavigation(navigationMemberInfo); - if (existingNavigation != null) - { - var fk = existingNavigation.ForeignKey; - if (fk.IsOwnership - || fk.DeclaringEntityType.Builder.HasNoRelationship(fk, fromDataAnnotation: true) == null) + var existingNavigation = entityType.FindNavigation(navigationMemberInfo); + if (existingNavigation != null) { - fk.Builder.HasNavigation( - (string)null, - existingNavigation.IsOnDependent, - fromDataAnnotation: true); + var fk = existingNavigation.ForeignKey; + if (fk.IsOwnership + || fk.DeclaringEntityType.Builder.HasNoRelationship(fk, fromDataAnnotation: true) == null) + { + fk.Builder.HasNavigation( + (string)null, + existingNavigation.IsOnDependent, + fromDataAnnotation: true); + } } - } - var existingAmbiguousNavigation = FindActualEntityType(ambiguousInverse.Value.Item2) - .FindNavigation(ambiguousInverse.Value.Item1); - if (existingAmbiguousNavigation != null) - { - var fk = existingAmbiguousNavigation.ForeignKey; - if (fk.IsOwnership - || fk.DeclaringEntityType.Builder.HasNoRelationship(fk, fromDataAnnotation: true) == null) + var existingAmbiguousNavigation = FindActualEntityType(ambiguousInverse.Value.Item2) + .FindNavigation(ambiguousInverse.Value.Item1); + if (existingAmbiguousNavigation != null) { - fk.Builder.HasNavigation( - (string)null, - existingAmbiguousNavigation.IsOnDependent, - fromDataAnnotation: true); + var fk = existingAmbiguousNavigation.ForeignKey; + if (fk.IsOwnership + || fk.DeclaringEntityType.Builder.HasNoRelationship(fk, fromDataAnnotation: true) == null) + { + fk.Builder.HasNavigation( + (string)null, + existingAmbiguousNavigation.IsOnDependent, + fromDataAnnotation: true); + } } + + return entityType.FindNavigation(navigationMemberInfo)?.ForeignKey.Builder; } - return entityType.FindNavigation(navigationMemberInfo)?.ForeignKey.Builder; + if (canBeSkipNavigation) + { + // TODO: Not sure what to do + } } var ownership = entityType.FindOwnership(); @@ -195,6 +209,7 @@ private IConventionForeignKeyBuilder ConfigureInverseNavigation( entityType, navigationMemberInfo, targetEntityTypeBuilder.Metadata, inverseNavigationPropertyInfo, ownership.PrincipalToDependent?.GetIdentifyingMemberInfo()); + return null; } @@ -206,21 +221,51 @@ private IConventionForeignKeyBuilder ConfigureInverseNavigation( entityType, navigationMemberInfo, targetEntityTypeBuilder.Metadata, inverseNavigationPropertyInfo, entityType.DefiningEntityType.GetRuntimeProperties()[entityType.DefiningNavigationName]); + return null; } - return entityType.Model.FindIsOwnedConfigurationSource(entityType.ClrType) != null - && !entityType.IsInOwnershipPath(targetEntityTypeBuilder.Metadata) - ? targetEntityTypeBuilder.HasOwnership( - entityTypeBuilder.Metadata.ClrType, - inverseNavigationPropertyInfo, - navigationMemberInfo, - fromDataAnnotation: true) - : targetEntityTypeBuilder.HasRelationship( - entityType, - inverseNavigationPropertyInfo, - navigationMemberInfo, - fromDataAnnotation: true); + if (entityType.Model.FindIsOwnedConfigurationSource(entityType.ClrType) != null + && !entityType.IsInOwnershipPath(targetEntityTypeBuilder.Metadata)) + { + return targetEntityTypeBuilder.HasOwnership( + entityTypeBuilder.Metadata.ClrType, + inverseNavigationPropertyInfo, + navigationMemberInfo, + fromDataAnnotation: true); + } + else + { + if (canBeNavigation) + { + var newForeignKeyBuilder = targetEntityTypeBuilder.HasRelationship( + entityType, + inverseNavigationPropertyInfo, + navigationMemberInfo, + fromDataAnnotation: true); + + if (newForeignKeyBuilder != null) + { + return newForeignKeyBuilder; + } + } + + if (canBeSkipNavigation + && navigationMemberInfo is PropertyInfo navigationPropertyInfo) + { + var navigationTargetType = navigationPropertyInfo.PropertyType.TryGetSequenceType(); + var inverseTargetType = inverseNavigationPropertyInfo.PropertyType.TryGetSequenceType(); + if (navigationTargetType == targetClrType + && inverseTargetType == entityType.ClrType) + { + entityTypeBuilder.HasSkipNavigation( + navigationPropertyInfo, targetEntityTypeBuilder.Metadata, + inverseNavigationPropertyInfo, collections: true, onDependent: false); + } + } + } + + return null; } /// @@ -276,7 +321,9 @@ public override void ProcessNavigationAdded( navigation.DeclaringEntityType.Builder, navigation.GetIdentifyingMemberInfo(), navigation.TargetEntityType.Builder, - attribute); + attribute, + canBeNavigation: true, + canBeSkipNavigation: false); if (newRelationshipBuilder == null) { diff --git a/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs b/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs index 72d995d9047..03825fc8d13 100644 --- a/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs +++ b/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs @@ -968,6 +968,19 @@ public virtual bool CanHaveNavigation([NotNull] string navigationName, Configura .Concat(Metadata.FindSkipNavigationsInHierarchy(navigationName)) .All(m => configurationSource.Overrides(m.GetConfigurationSource())); + /// + /// 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 bool CanHaveSkipNavigation([NotNull] string skipNavigationName, ConfigurationSource? configurationSource) + => !IsIgnored(skipNavigationName, configurationSource) + && Metadata.FindPropertiesInHierarchy(skipNavigationName).Cast() + .Concat(Metadata.FindServicePropertiesInHierarchy(skipNavigationName)) + .Concat(Metadata.FindNavigationsInHierarchy(skipNavigationName)) + .All(m => configurationSource.Overrides(m.GetConfigurationSource())); + /// /// 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 @@ -4871,6 +4884,18 @@ bool IConventionEntityTypeBuilder.CanHaveNavigation(string navigationName, bool navigationName, 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 + /// 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. + /// + [DebuggerStepThrough] + bool IConventionEntityTypeBuilder.CanHaveSkipNavigation(string skipNavigationName, bool fromDataAnnotation) + => CanHaveSkipNavigation( + skipNavigationName, + fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + /// [DebuggerStepThrough] IConventionSkipNavigationBuilder IConventionEntityTypeBuilder.HasSkipNavigation( diff --git a/test/EFCore.Specification.Tests/Query/ManyToManyQueryFixtureBase.cs b/test/EFCore.Specification.Tests/Query/ManyToManyQueryFixtureBase.cs index 6668dfe248b..ffecf56efb3 100644 --- a/test/EFCore.Specification.Tests/Query/ManyToManyQueryFixtureBase.cs +++ b/test/EFCore.Specification.Tests/Query/ManyToManyQueryFixtureBase.cs @@ -183,9 +183,6 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con l => l.HasOne(x => x.One).WithMany(e => e.JoinThreePayloadFull)) .HasKey(e => new { e.OneId, e.ThreeId }); - // Nav:2 Payload:No Join:Shared Extra:None - modelBuilder.Entity().HasMany(e => e.TwoSkipShared).WithMany(e => e.OneSkipShared); - // Nav:4 Payload:Yes Join:Shared Extra:None modelBuilder.Entity() .HasMany(e => e.ThreeSkipPayloadFullShared) diff --git a/test/EFCore.Specification.Tests/TestModels/ManyToManyModel/EntityOne.cs b/test/EFCore.Specification.Tests/TestModels/ManyToManyModel/EntityOne.cs index 80395c1d2c9..83e17c5f437 100644 --- a/test/EFCore.Specification.Tests/TestModels/ManyToManyModel/EntityOne.cs +++ b/test/EFCore.Specification.Tests/TestModels/ManyToManyModel/EntityOne.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Collections.ObjectModel; +using System.ComponentModel.DataAnnotations.Schema; namespace Microsoft.EntityFrameworkCore.TestModels.ManyToManyModel { @@ -21,6 +22,7 @@ public class EntityOne public virtual ICollection JoinThreePayloadFull { get; } = new ObservableCollection(); // #21684 + [InverseProperty("OneSkipShared")] public virtual ICollection TwoSkipShared { get; } = new ObservableCollection(); // #21684 public virtual ICollection ThreeSkipPayloadFullShared { get; } = new ObservableCollection(); // #21684 diff --git a/test/EFCore.Specification.Tests/TestModels/ManyToManyModel/EntityTwo.cs b/test/EFCore.Specification.Tests/TestModels/ManyToManyModel/EntityTwo.cs index 4118927e8de..9473feb2bbd 100644 --- a/test/EFCore.Specification.Tests/TestModels/ManyToManyModel/EntityTwo.cs +++ b/test/EFCore.Specification.Tests/TestModels/ManyToManyModel/EntityTwo.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Collections.ObjectModel; +using System.ComponentModel.DataAnnotations.Schema; namespace Microsoft.EntityFrameworkCore.TestModels.ManyToManyModel { @@ -25,6 +26,7 @@ public class EntityTwo public virtual ICollection SelfSkipSharedLeft { get; } = new ObservableCollection(); // #21684 public virtual ICollection SelfSkipSharedRight { get; } = new ObservableCollection(); // #21684 + [InverseProperty("TwoSkipShared")] public virtual ICollection OneSkipShared { get; } = new ObservableCollection(); // #21684 public virtual ICollection CompositeKeySkipShared { get; }