diff --git a/src/EFCore/Metadata/Builders/CollectionCollectionBuilder.cs b/src/EFCore/Metadata/Builders/CollectionCollectionBuilder.cs index a4db5c04972..f38487719a9 100644 --- a/src/EFCore/Metadata/Builders/CollectionCollectionBuilder.cs +++ b/src/EFCore/Metadata/Builders/CollectionCollectionBuilder.cs @@ -4,6 +4,7 @@ using System; using System.ComponentModel; using JetBrains.Annotations; +using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Metadata.Internal; using Microsoft.EntityFrameworkCore.Utilities; @@ -94,13 +95,19 @@ public virtual EntityTypeBuilder UsingEntity( [NotNull] Func configureRight, [NotNull] Func configureLeft) { + if (((Model)LeftEntityType.Model).IsShared(joinEntity)) + { + throw new InvalidOperationException( + CoreStrings.DoNotUseUsingEntityOnSharedClrType(joinEntity.GetType().Name)); + } + var existingAssociationEntityType = (EntityType) (LeftNavigation.ForeignKey?.DeclaringEntityType ?? RightNavigation.ForeignKey?.DeclaringEntityType); if (existingAssociationEntityType != null) { ModelBuilder.RemoveAssociationEntityIfAutomaticallyCreated( - existingAssociationEntityType, false, ConfigurationSource.Explicit); + existingAssociationEntityType, removeSkipNavigations: false, ConfigurationSource.Explicit); } var entityTypeBuilder = new EntityTypeBuilder( diff --git a/src/EFCore/Metadata/Builders/CollectionCollectionBuilder`.cs b/src/EFCore/Metadata/Builders/CollectionCollectionBuilder`.cs index b0db5b3bf49..e146372b721 100644 --- a/src/EFCore/Metadata/Builders/CollectionCollectionBuilder`.cs +++ b/src/EFCore/Metadata/Builders/CollectionCollectionBuilder`.cs @@ -3,6 +3,7 @@ using System; using JetBrains.Annotations; +using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Metadata.Internal; @@ -51,13 +52,19 @@ public virtual EntityTypeBuilder UsingEntity, ReferenceCollectionBuilder> configureLeft) where TAssociationEntity : class { + if (((Model)LeftEntityType.Model).IsShared(typeof(TAssociationEntity))) + { + throw new InvalidOperationException( + CoreStrings.DoNotUseUsingEntityOnSharedClrType(typeof(TAssociationEntity).Name)); + } + var existingAssociationEntityType = (EntityType) (LeftNavigation.ForeignKey?.DeclaringEntityType ?? RightNavigation.ForeignKey?.DeclaringEntityType); if (existingAssociationEntityType != null) { ModelBuilder.RemoveAssociationEntityIfAutomaticallyCreated( - existingAssociationEntityType, false, ConfigurationSource.Explicit); + existingAssociationEntityType, removeSkipNavigations: false, ConfigurationSource.Explicit); } var entityTypeBuilder = new EntityTypeBuilder( diff --git a/src/EFCore/Metadata/Builders/CollectionNavigationBuilder.cs b/src/EFCore/Metadata/Builders/CollectionNavigationBuilder.cs index 7c36f3f3a0c..a8d5cc64242 100644 --- a/src/EFCore/Metadata/Builders/CollectionNavigationBuilder.cs +++ b/src/EFCore/Metadata/Builders/CollectionNavigationBuilder.cs @@ -205,11 +205,17 @@ public virtual CollectionCollectionBuilder WithMany([NotNull] string navigationN } var leftName = Builder?.Metadata.PrincipalToDependent.Name; - return new CollectionCollectionBuilder( - RelatedEntityType, - DeclaringEntityType, - WithLeftManyNavigation(navigationName), - WithRightManyNavigation(navigationName, leftName)); + var collectionCollectionBuilder = + new CollectionCollectionBuilder( + RelatedEntityType, + DeclaringEntityType, + WithLeftManyNavigation(navigationName), + WithRightManyNavigation(navigationName, leftName)); + + collectionCollectionBuilder.LeftNavigation + .SetInverse(collectionCollectionBuilder.RightNavigation); + + return collectionCollectionBuilder; } /// diff --git a/src/EFCore/Metadata/Builders/CollectionNavigationBuilder`.cs b/src/EFCore/Metadata/Builders/CollectionNavigationBuilder`.cs index c5abc27f5a6..07f9336abfa 100644 --- a/src/EFCore/Metadata/Builders/CollectionNavigationBuilder`.cs +++ b/src/EFCore/Metadata/Builders/CollectionNavigationBuilder`.cs @@ -95,11 +95,17 @@ public virtual ReferenceCollectionBuilder WithOne( public new virtual CollectionCollectionBuilder WithMany([NotNull] string navigationName) { var leftName = Builder?.Metadata.PrincipalToDependent.Name; - return new CollectionCollectionBuilder( - RelatedEntityType, - DeclaringEntityType, - WithLeftManyNavigation(navigationName), - WithRightManyNavigation(navigationName, leftName)); + var collectionCollectionBuilder = + new CollectionCollectionBuilder( + RelatedEntityType, + DeclaringEntityType, + WithLeftManyNavigation(navigationName), + WithRightManyNavigation(navigationName, leftName)); + + collectionCollectionBuilder.LeftNavigation + .SetInverse(collectionCollectionBuilder.RightNavigation); + + return collectionCollectionBuilder; } /// @@ -126,11 +132,17 @@ public virtual CollectionCollectionBuilder WithMany( } var leftName = Builder?.Metadata.PrincipalToDependent.Name; - return new CollectionCollectionBuilder( - RelatedEntityType, - DeclaringEntityType, - WithLeftManyNavigation(navigationExpression.GetMemberAccess()), - WithRightManyNavigation(navigationExpression.GetMemberAccess(), leftName)); + var collectionCollectionBuilder = + new CollectionCollectionBuilder( + RelatedEntityType, + DeclaringEntityType, + WithLeftManyNavigation(navigationExpression.GetMemberAccess()), + WithRightManyNavigation(navigationExpression.GetMemberAccess(), leftName)); + + collectionCollectionBuilder.LeftNavigation + .SetInverse(collectionCollectionBuilder.RightNavigation); + + return collectionCollectionBuilder; } } } diff --git a/src/EFCore/Metadata/Conventions/BackingFieldConvention.cs b/src/EFCore/Metadata/Conventions/BackingFieldConvention.cs index b37df7903fc..a595f31b8e9 100644 --- a/src/EFCore/Metadata/Conventions/BackingFieldConvention.cs +++ b/src/EFCore/Metadata/Conventions/BackingFieldConvention.cs @@ -81,7 +81,7 @@ private FieldInfo GetFieldToSet(IConventionPropertyBase propertyBase) if (propertyBase == null || !ConfigurationSource.Convention.Overrides(propertyBase.GetFieldInfoConfigurationSource()) || propertyBase.IsIndexerProperty() - || (propertyBase.PropertyInfo == null && propertyBase.FieldInfo == null)) + || propertyBase.IsShadowProperty()) { return null; } diff --git a/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs b/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs index ca9a0e4e051..9ad841f3aaf 100644 --- a/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs +++ b/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs @@ -200,7 +200,7 @@ public virtual ConventionSet CreateConventionSet() conventionSet.NavigationRemovedConventions.Add(relationshipDiscoveryConvention); - conventionSet.SkipNavigationAddedConventions.Add(new ManyToManyConvention(Dependencies)); + conventionSet.SkipNavigationAddedConventions.Add(new ManyToManyAssociationEntityTypeConvention(Dependencies)); conventionSet.IndexAddedConventions.Add(foreignKeyIndexConvention); diff --git a/src/EFCore/Metadata/Conventions/ManyToManyConvention.cs b/src/EFCore/Metadata/Conventions/ManyToManyAssociationEntityTypeConvention.cs similarity index 73% rename from src/EFCore/Metadata/Conventions/ManyToManyConvention.cs rename to src/EFCore/Metadata/Conventions/ManyToManyAssociationEntityTypeConvention.cs index ae9881575ba..91f1a926aa0 100644 --- a/src/EFCore/Metadata/Conventions/ManyToManyConvention.cs +++ b/src/EFCore/Metadata/Conventions/ManyToManyAssociationEntityTypeConvention.cs @@ -13,16 +13,15 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Conventions /// /// A convention which looks for matching skip navigations and automatically creates /// a many-to-many association entity with suitable foreign keys, sets the two - /// matching skip navigations to use those foreign keys and makes them inverses of - /// one another. + /// matching skip navigations to use those foreign keys. /// - public class ManyToManyConvention : ISkipNavigationAddedConvention + public class ManyToManyAssociationEntityTypeConvention : ISkipNavigationAddedConvention { /// - /// Creates a new instance of . + /// Creates a new instance of . /// /// Parameter object containing dependencies for this convention. - public ManyToManyConvention([NotNull] ProviderConventionSetBuilderDependencies dependencies) + public ManyToManyAssociationEntityTypeConvention([NotNull] ProviderConventionSetBuilderDependencies dependencies) { Dependencies = dependencies; } @@ -41,8 +40,8 @@ public virtual void ProcessSkipNavigationAdded( IConventionSkipNavigationBuilder skipNavigationBuilder, IConventionContext context) { - Check.NotNull(skipNavigationBuilder, "skipNavigationBuilder"); - Check.NotNull(context, "context"); + Check.NotNull(skipNavigationBuilder, nameof(skipNavigationBuilder)); + Check.NotNull(context, nameof(context)); var skipNavigation = skipNavigationBuilder.Metadata; if (skipNavigation.ForeignKey != null @@ -55,13 +54,10 @@ public virtual void ProcessSkipNavigationAdded( return; } - var matchingSkipNavigation = skipNavigation.TargetEntityType - .GetSkipNavigations() - .FirstOrDefault(sn => sn.TargetEntityType == skipNavigation.DeclaringEntityType); - - if (matchingSkipNavigation == null - || matchingSkipNavigation.ForeignKey != null - || !matchingSkipNavigation.IsCollection) + var inverseSkipNavigation = skipNavigation.Inverse; + if (inverseSkipNavigation == null + || inverseSkipNavigation.ForeignKey != null + || !inverseSkipNavigation.IsCollection) { // do not create an automatic many-to-many association entity type if // the matching skip navigation is already "in use" (i.e. @@ -70,9 +66,9 @@ public virtual void ProcessSkipNavigationAdded( } var model = (Model)skipNavigation.DeclaringEntityType.Model; - model.Builder.AssociationEntity( + model.Builder.HasAutomaticAssociationEntity( (SkipNavigation)skipNavigation, - (SkipNavigation)matchingSkipNavigation, + (SkipNavigation)inverseSkipNavigation, ConfigurationSource.Convention); } } diff --git a/src/EFCore/Metadata/Conventions/RelationshipDiscoveryConvention.cs b/src/EFCore/Metadata/Conventions/RelationshipDiscoveryConvention.cs index 40501637151..36db5bf42e3 100644 --- a/src/EFCore/Metadata/Conventions/RelationshipDiscoveryConvention.cs +++ b/src/EFCore/Metadata/Conventions/RelationshipDiscoveryConvention.cs @@ -241,21 +241,25 @@ private static IReadOnlyList RemoveIncompatibleWithExisti var navigationProperty = relationshipCandidate.NavigationProperties[0]; var navigationPropertyName = navigationProperty.GetSimpleMemberName(); var existingNavigation = entityType.FindNavigation(navigationPropertyName); - if (existingNavigation != null - && (existingNavigation.DeclaringEntityType != entityType - || existingNavigation.TargetEntityType != targetEntityType)) + if (existingNavigation != null) { - relationshipCandidate.NavigationProperties.Remove(navigationProperty); - continue; + if (existingNavigation.DeclaringEntityType != entityType + || existingNavigation.TargetEntityType != targetEntityType) + { + relationshipCandidate.NavigationProperties.Remove(navigationProperty); + continue; + } } - - var existingSkipNavigation = entityType.FindSkipNavigation(navigationPropertyName); - if (existingSkipNavigation != null - && (existingSkipNavigation.DeclaringEntityType != entityType - || existingSkipNavigation.TargetEntityType != targetEntityType)) + else { - relationshipCandidate.NavigationProperties.Remove(navigationProperty); - continue; + var existingSkipNavigation = entityType.FindSkipNavigation(navigationPropertyName); + if (existingSkipNavigation != null + && (existingSkipNavigation.DeclaringEntityType != entityType + || existingSkipNavigation.TargetEntityType != targetEntityType)) + { + relationshipCandidate.NavigationProperties.Remove(navigationProperty); + continue; + } } if (relationshipCandidate.NavigationProperties.Count == 1 @@ -583,74 +587,14 @@ private void CreateRelationships( foreach (var navigationProperty in relationshipCandidate.NavigationProperties.ToList()) { - var navigationPropertyName = navigationProperty.GetSimpleMemberName(); - var existingNavigation = entityType.FindDeclaredNavigation(navigationPropertyName); - if (existingNavigation != null) - { - if (existingNavigation.ForeignKey.DeclaringEntityType.Builder - .HasNoRelationship(existingNavigation.ForeignKey) == null - && existingNavigation.ForeignKey.Builder.HasNavigation( - (string)null, existingNavigation.IsOnDependent) == null) - { - // Navigations of higher configuration source are not ambiguous - relationshipCandidate.NavigationProperties.Remove(navigationProperty); - } - } - else - { - var associationEntityType = (EntityType)entityType - .FindDeclaredSkipNavigation(navigationPropertyName)? - .ForeignKey?.DeclaringEntityType; - if (associationEntityType != null) - { - var modelBuilder = associationEntityType.Model.Builder; - // The PropertyInfo underlying this skip navigation has become - // ambiguous since we used it, so remove the association entity - // if it was automatically created. - if (modelBuilder.RemoveAssociationEntityIfAutomaticallyCreated( - associationEntityType, true, ConfigurationSource.Convention) == null) - { - // Navigations of higher configuration source are not ambiguous - relationshipCandidate.NavigationProperties.Remove(navigationProperty); - } - } - } + RemoveHigherConfigurationSourceNavigation( + navigationProperty, entityType, relationshipCandidate.NavigationProperties); } foreach (var inverseProperty in relationshipCandidate.InverseProperties.ToList()) { - var inversePropertyName = inverseProperty.GetSimpleMemberName(); - var existingInverse = targetEntityType.FindDeclaredNavigation(inversePropertyName); - if (existingInverse != null) - { - if (existingInverse.ForeignKey.DeclaringEntityType.Builder - .HasNoRelationship(existingInverse.ForeignKey) == null - && existingInverse.ForeignKey.Builder.HasNavigation( - (string)null, existingInverse.IsOnDependent) == null) - { - // Navigations of higher configuration source are not ambiguous - relationshipCandidate.InverseProperties.Remove(inverseProperty); - } - } - else - { - var associationEntityType = (EntityType)targetEntityType - .FindDeclaredSkipNavigation(inversePropertyName)? - .ForeignKey?.DeclaringEntityType; - if (associationEntityType != null) - { - var modelBuilder = associationEntityType.Model.Builder; - // The PropertyInfo underlying this skip navigation has become - // ambiguous since we used it, so remove the association entity - // if it was automatically created. - if (modelBuilder.RemoveAssociationEntityIfAutomaticallyCreated( - associationEntityType, true, ConfigurationSource.Convention) == null) - { - // Navigations of higher configuration source are not ambiguous - relationshipCandidate.NavigationProperties.Remove(inverseProperty); - } - } - } + RemoveHigherConfigurationSourceNavigation( + inverseProperty, targetEntityType, relationshipCandidate.InverseProperties); } if (!isAmbiguousOnBase) @@ -783,6 +727,45 @@ private void CreateRelationships( } } + private void RemoveHigherConfigurationSourceNavigation( + PropertyInfo navigationProperty, + IConventionEntityType declaringEntityType, + List toRemoveFrom) + { + var navigationPropertyName = navigationProperty.GetSimpleMemberName(); + var existingNavigation = declaringEntityType.FindDeclaredNavigation(navigationPropertyName); + if (existingNavigation != null) + { + if (existingNavigation.ForeignKey.DeclaringEntityType.Builder + .HasNoRelationship(existingNavigation.ForeignKey) == null + && existingNavigation.ForeignKey.Builder.HasNavigation( + (string)null, existingNavigation.IsOnDependent) == null) + { + // Navigations of higher configuration source are not ambiguous + toRemoveFrom.Remove(navigationProperty); + } + } + else + { + var associationEntityType = (EntityType)declaringEntityType + .FindDeclaredSkipNavigation(navigationPropertyName)? + .ForeignKey?.DeclaringEntityType; + if (associationEntityType != null) + { + var modelBuilder = associationEntityType.Model.Builder; + // The PropertyInfo underlying this skip navigation has become + // ambiguous since we used it, so remove the association entity + // if it was automatically created. + if (modelBuilder.RemoveAssociationEntityIfAutomaticallyCreated( + associationEntityType, removeSkipNavigations: true, ConfigurationSource.Convention) == null) + { + // Navigations of higher configuration source are not ambiguous + toRemoveFrom.Remove(navigationProperty); + } + } + } + } + private void HasManyToManyRelationship( IConventionEntityTypeBuilder entityTypeBuilder, RelationshipCandidate relationshipCandidate, @@ -814,15 +797,6 @@ private void HasManyToManyRelationship( return; } - var leftSkipNavigation = leftEntityType.FindSkipNavigation(navigation); - var rightSkipNavigation = rightEntityType.FindSkipNavigation(inverse); - if (leftSkipNavigation != null - || rightSkipNavigation != null) - { - // skip navigations have already been configured using these properties - return; - } - var navigationName = navigation.GetSimpleMemberName(); var inverseName = inverse.GetSimpleMemberName(); if (((EntityType)leftEntityType) @@ -834,37 +808,31 @@ private void HasManyToManyRelationship( return; } - leftSkipNavigation = leftEntityType.AddSkipNavigation( - navigationName, navigation, rightEntityType, - collection: true, onDependent: false, fromDataAnnotation: false); + var leftSkipNavigation = leftEntityType.FindSkipNavigation(navigation) ?? + leftEntityType.AddSkipNavigation( + navigationName, navigation, rightEntityType, + collection: true, onDependent: false, fromDataAnnotation: false); if (leftSkipNavigation == null) { return; } - rightSkipNavigation = rightEntityType.AddSkipNavigation( - inverseName, inverse, leftEntityType, - collection: true, onDependent: false, fromDataAnnotation: false); + var rightSkipNavigation = rightEntityType.FindSkipNavigation(inverse) ?? + rightEntityType.AddSkipNavigation( + inverseName, inverse, leftEntityType, + collection: true, onDependent: false, fromDataAnnotation: false); if (rightSkipNavigation == null) { - // Failed to create the right skip navigation - so remove - // the left skip navigation we just created as well. - leftEntityType.RemoveSkipNavigation(leftSkipNavigation); return; } - var associationEntityTypeBuilder = - ((InternalModelBuilder)entityTypeBuilder.ModelBuilder).AssociationEntity( - (SkipNavigation)leftSkipNavigation, - (SkipNavigation)rightSkipNavigation, - ConfigurationSource.Convention); - if (associationEntityTypeBuilder == null) - { - // Failed to create the association entity type - so remove - // both the skip navigations we just created as well. - leftEntityType.RemoveSkipNavigation(leftSkipNavigation); - rightEntityType.RemoveSkipNavigation(rightSkipNavigation); - } + // also sets rightSkipNavigation's inverse + leftSkipNavigation.Builder.HasInverse(rightSkipNavigation, fromDataAnnotation: false); + + ((InternalModelBuilder)entityTypeBuilder.ModelBuilder).HasAutomaticAssociationEntity( + (SkipNavigation)leftSkipNavigation, + (SkipNavigation)rightSkipNavigation, + ConfigurationSource.Convention); } /// diff --git a/src/EFCore/Metadata/IConventionModel.cs b/src/EFCore/Metadata/IConventionModel.cs index 12ffd3b156e..7c7e66c82d2 100644 --- a/src/EFCore/Metadata/IConventionModel.cs +++ b/src/EFCore/Metadata/IConventionModel.cs @@ -135,6 +135,13 @@ IConventionEntityType FindEntityType( /// The removed ignored type name. string RemoveIgnored([NotNull] string typeName); + /// + /// Gets whether the CLR type is used by shared entities in the model. + /// + /// The CLR type. + /// Whether the CLR type is used by shared entities in the model. + bool IsShared([NotNull] Type clrType); + /// /// Indicates whether the given entity type name is ignored. /// diff --git a/src/EFCore/Metadata/Internal/InternalModelBuilder.cs b/src/EFCore/Metadata/Internal/InternalModelBuilder.cs index 3e2bb75ae63..bd76038e722 100644 --- a/src/EFCore/Metadata/Internal/InternalModelBuilder.cs +++ b/src/EFCore/Metadata/Internal/InternalModelBuilder.cs @@ -220,19 +220,21 @@ private InternalEntityTypeBuilder Entity( /// 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 InternalEntityTypeBuilder AssociationEntity( + public virtual InternalEntityTypeBuilder HasAutomaticAssociationEntity( [NotNull] SkipNavigation leftSkipNavigation, [NotNull] SkipNavigation rightSkipNavigation, ConfigurationSource configurationSource) { + // This API is used solely to create association entities automatically + // i.e. through convention. Fluent-API creation of association entities + // is handled separately. Check.NotNull(leftSkipNavigation, nameof(leftSkipNavigation)); Check.NotNull(rightSkipNavigation, nameof(rightSkipNavigation)); var leftEntityType = leftSkipNavigation.DeclaringEntityType; var rightEntityType = rightSkipNavigation.DeclaringEntityType; - if (leftSkipNavigation.TargetEntityType != rightEntityType - || rightSkipNavigation.TargetEntityType != leftEntityType) + if (leftSkipNavigation.Inverse == null) { if (configurationSource != ConfigurationSource.Explicit) { @@ -240,13 +242,65 @@ public virtual InternalEntityTypeBuilder AssociationEntity( } throw new InvalidOperationException( - CoreStrings.InvalidSkipNavigationsForAssociationEntityType( + CoreStrings.SkipNavigationForAssociationEntityTypeHasNoInverse( leftEntityType.Name, leftSkipNavigation.Name, - leftSkipNavigation.TargetEntityType.Name, + rightEntityType.Name, + rightSkipNavigation.Name)); + } + + if (rightSkipNavigation.Inverse == null) + { + if (configurationSource != ConfigurationSource.Explicit) + { + return null; + } + + throw new InvalidOperationException( + CoreStrings.SkipNavigationForAssociationEntityTypeHasNoInverse( + rightEntityType.Name, + rightSkipNavigation.Name, + leftEntityType.Name, + leftSkipNavigation.Name)); + } + + if (leftSkipNavigation.Inverse != rightSkipNavigation) + { + if (configurationSource != ConfigurationSource.Explicit) + { + return null; + } + + throw new InvalidOperationException( + CoreStrings.SkipNavigationForAssociationEntityTypeWrongInverse( + leftEntityType.Name, + leftSkipNavigation.Name, + leftSkipNavigation.Inverse.DeclaringEntityType.Name, + leftSkipNavigation.Inverse.Name, + rightEntityType.Name, + rightSkipNavigation.Name)); + } + + if (rightSkipNavigation.Inverse != leftSkipNavigation) + { + if (configurationSource != ConfigurationSource.Explicit) + { + return null; + } + + throw new InvalidOperationException( + CoreStrings.SkipNavigationForAssociationEntityTypeWrongInverse( rightEntityType.Name, rightSkipNavigation.Name, - rightSkipNavigation.TargetEntityType.Name)); + rightSkipNavigation.Inverse.DeclaringEntityType.Name, + rightSkipNavigation.Inverse.Name, + leftEntityType.Name, + leftSkipNavigation.Name)); + } + + if (leftSkipNavigation.AssociationEntityType != null) + { + return leftSkipNavigation.AssociationEntityType.Builder; } // create the association entity type @@ -305,9 +359,8 @@ public virtual InternalEntityTypeBuilder AssociationEntity( associationEntityTypeName)); } - leftSkipNavigation.SetForeignKey(leftForeignKey, configurationSource); - rightSkipNavigation.SetForeignKey(rightForeignKey, configurationSource); - leftSkipNavigation.Builder.HasInverse(rightSkipNavigation, configurationSource); + leftSkipNavigation.Builder.HasForeignKey(leftForeignKey, configurationSource); + rightSkipNavigation.Builder.HasForeignKey(rightForeignKey, configurationSource); // Creating the primary key below also negates the need for an index on // the properties of leftForeignKey - that index is automatically removed. @@ -352,6 +405,7 @@ private static ForeignKey CreateSkipNavigationForeignKey( dependentEndForeignKeyPropertyNames, principalKey, configurationSource) + .IsUnique(false, configurationSource) .Metadata; } @@ -377,19 +431,17 @@ public virtual InternalModelBuilder RemoveAssociationEntityIfAutomaticallyCreate "Automatically created association entity types should have exactly 2 foreign keys"); foreach (var fk in associationEntityType.GetForeignKeys()) { - var principalEntityType = fk.PrincipalEntityType; - var skipNavigation = principalEntityType - .GetSkipNavigations().FirstOrDefault(sn => fk == sn.ForeignKey); + var skipNavigation = fk.GetReferencingSkipNavigations().FirstOrDefault(); if (skipNavigation != null) { skipNavigation.SetForeignKey(null, configurationSource); skipNavigation.SetInverse(null, configurationSource); if (removeSkipNavigations - && principalEntityType.Builder + && fk.PrincipalEntityType.Builder .CanRemoveSkipNavigation(skipNavigation, configurationSource)) { - principalEntityType.RemoveSkipNavigation(skipNavigation); + fk.PrincipalEntityType.RemoveSkipNavigation(skipNavigation); } } } diff --git a/src/EFCore/Metadata/Internal/InternalSkipNavigationBuilder.cs b/src/EFCore/Metadata/Internal/InternalSkipNavigationBuilder.cs index dcb0d22b62d..96c39ea766f 100644 --- a/src/EFCore/Metadata/Internal/InternalSkipNavigationBuilder.cs +++ b/src/EFCore/Metadata/Internal/InternalSkipNavigationBuilder.cs @@ -94,6 +94,17 @@ public virtual InternalSkipNavigationBuilder HasForeignKey([CanBeNull] ForeignKe foreignKey.UpdateConfigurationSource(configurationSource); } + if (foreignKey != Metadata.ForeignKey + && Metadata.AssociationEntityType?.IsAutomaticallyCreatedAssociationEntityType == true) + { + // Have reset the foreign key of a skip navigation on one side of an + // automatically created association entity. That entity is only + // useful if both sides are configured - so remove that + // entity. This will remove the Inverse's foreign key as well. + Metadata.AssociationEntityType.Model.Builder.RemoveAssociationEntityIfAutomaticallyCreated( + Metadata.AssociationEntityType, removeSkipNavigations: false, configurationSource); + } + Metadata.SetForeignKey(foreignKey, configurationSource); return this; } @@ -122,7 +133,7 @@ public virtual bool CanSetForeignKey([CanBeNull] ForeignKey foreignKey, Configur return (Metadata.DeclaringEntityType == (Metadata.IsOnDependent ? foreignKey.DeclaringEntityType : foreignKey.PrincipalEntityType)) && (Metadata.Inverse?.AssociationEntityType == null - || Metadata.Inverse?.AssociationEntityType?.IsAutomaticallyCreatedAssociationEntityType == true + || Metadata.Inverse.AssociationEntityType.IsAutomaticallyCreatedAssociationEntityType == true || Metadata.Inverse.AssociationEntityType == (Metadata.IsOnDependent ? foreignKey.PrincipalEntityType : foreignKey.DeclaringEntityType)); } diff --git a/src/EFCore/Metadata/Internal/Model.cs b/src/EFCore/Metadata/Internal/Model.cs index 6618637e937..9126652aaf0 100644 --- a/src/EFCore/Metadata/Internal/Model.cs +++ b/src/EFCore/Metadata/Internal/Model.cs @@ -754,6 +754,15 @@ public virtual bool IsIgnored(string name) public virtual bool IsIgnored([NotNull] Type type) => FindIgnoredConfigurationSource(GetDisplayName(type)) != null; + /// + /// 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 IsShared([NotNull] Type type) + => _sharedEntityClrTypes.Contains(type); + /// /// 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 @@ -1181,5 +1190,13 @@ IConventionEntityType IConventionModel.RemoveEntityType(IConventionEntityType en /// string IConventionModel.AddIgnored(string name, bool fromDataAnnotation) => AddIgnored(name, 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. + /// + bool IConventionModel.IsShared(Type clrType) => IsShared(clrType); } } diff --git a/src/EFCore/Metadata/Internal/SkipNavigation.cs b/src/EFCore/Metadata/Internal/SkipNavigation.cs index b278d465d38..8cd418c6cd4 100644 --- a/src/EFCore/Metadata/Internal/SkipNavigation.cs +++ b/src/EFCore/Metadata/Internal/SkipNavigation.cs @@ -193,23 +193,10 @@ public virtual ForeignKey SetForeignKey([CanBeNull] ForeignKey foreignKey, Confi if (Inverse?.AssociationEntityType != null && Inverse.AssociationEntityType != AssociationEntityType) { - if (!Inverse.AssociationEntityType.IsAutomaticallyCreatedAssociationEntityType) - { - throw new InvalidOperationException(CoreStrings.SkipInverseMismatchedForeignKey( - foreignKey.Properties.Format(), - Name, AssociationEntityType.DisplayName(), - Inverse.Name, Inverse.AssociationEntityType.DisplayName())); - } - - // Have reset the foreign key of a skip navigation on one side of an - // automatically created association entity. That entity is only - // useful if both sides are configured - so remove that - // entity and set the Inverse skip navigation to have no foreign key. - // If the user wants to set the Inverse's foreign key to also point - // to the new association entity they will need to do that manually. - AssociationEntityType.Model.Builder.RemoveAssociationEntityIfAutomaticallyCreated( - AssociationEntityType, false, configurationSource); - Inverse.SetForeignKey(null, configurationSource); + throw new InvalidOperationException(CoreStrings.SkipInverseMismatchedForeignKey( + foreignKey.Properties.Format(), + Name, AssociationEntityType.DisplayName(), + Inverse.Name, Inverse.AssociationEntityType.DisplayName())); } return isChanging diff --git a/src/EFCore/Properties/CoreStrings.Designer.cs b/src/EFCore/Properties/CoreStrings.Designer.cs index 1ede2c33080..0cda8ceeb37 100644 --- a/src/EFCore/Properties/CoreStrings.Designer.cs +++ b/src/EFCore/Properties/CoreStrings.Designer.cs @@ -2709,12 +2709,20 @@ public static string UnhandledNavigationBase([CanBeNull] object type) type); /// - /// Cannot create an association entity type using skip navigations '{leftEntityType}.{leftSkipNavigationName}' which targets entity type '{leftTargetEntityType}', and '{rightEntityType}.{rightSkipNavigationName}' which targets '{rightTargetEntityType}'. They should target one another. + /// Cannot use using skip navigation '{declaringEntityType}.{skipNavigationName}' to create an association entity type. Its inverse is not set. The inverse should be the other skip navigation, '{otherEntityType}.{otherSkipNavigationName}'. /// - public static string InvalidSkipNavigationsForAssociationEntityType([CanBeNull] object leftEntityType, [CanBeNull] object leftSkipNavigationName, [CanBeNull] object leftTargetEntityType, [CanBeNull] object rightEntityType, [CanBeNull] object rightSkipNavigationName, [CanBeNull] object rightTargetEntityType) + public static string SkipNavigationForAssociationEntityTypeHasNoInverse([CanBeNull] object declaringEntityType, [CanBeNull] object skipNavigationName, [CanBeNull] object otherEntityType, [CanBeNull] object otherSkipNavigationName) => string.Format( - GetString("InvalidSkipNavigationsForAssociationEntityType", nameof(leftEntityType), nameof(leftSkipNavigationName), nameof(leftTargetEntityType), nameof(rightEntityType), nameof(rightSkipNavigationName), nameof(rightTargetEntityType)), - leftEntityType, leftSkipNavigationName, leftTargetEntityType, rightEntityType, rightSkipNavigationName, rightTargetEntityType); + GetString("SkipNavigationForAssociationEntityTypeHasNoInverse", nameof(declaringEntityType), nameof(skipNavigationName), nameof(otherEntityType), nameof(otherSkipNavigationName)), + declaringEntityType, skipNavigationName, otherEntityType, otherSkipNavigationName); + + /// + /// Cannot use using skip navigation '{declaringEntityType}.{skipNavigationName}' to create an association entity type. Its inverse is '{inverseEntityType}.{inverseSkipNavigationName}', but it should be '{otherEntityType}.{otherSkipNavigationName}'. + /// + public static string SkipNavigationForAssociationEntityTypeWrongInverse([CanBeNull] object declaringEntityType, [CanBeNull] object skipNavigationName, [CanBeNull] object inverseEntityType, [CanBeNull] object inverseSkipNavigationName, [CanBeNull] object otherEntityType, [CanBeNull] object otherSkipNavigationName) + => string.Format( + GetString("SkipNavigationForAssociationEntityTypeWrongInverse", nameof(declaringEntityType), nameof(skipNavigationName), nameof(inverseEntityType), nameof(inverseSkipNavigationName), nameof(otherEntityType), nameof(otherSkipNavigationName)), + declaringEntityType, skipNavigationName, inverseEntityType, inverseSkipNavigationName, otherEntityType, otherSkipNavigationName); /// /// Cannot create a foreign key for skip navigation '{declaringEntityType}.{skipNavigationName}' on association entity type '{associationEntityType}'. Ensure '{declaringEntityType}' has a primary key and is not ignored in the model. @@ -2724,6 +2732,14 @@ public static string UnableToCreateSkipNavigationForeignKeyOnAssociationEntityTy GetString("UnableToCreateSkipNavigationForeignKeyOnAssociationEntityType", nameof(declaringEntityType), nameof(skipNavigationName), nameof(associationEntityType)), declaringEntityType, skipNavigationName, associationEntityType); + /// + /// Cannot use UsingEntity() passing type '{clrType}' because the model contains shared entity type(s) with same type. Use a type which uniquely defines an entity type. + /// + public static string DoNotUseUsingEntityOnSharedClrType([CanBeNull] object clrType) + => string.Format( + GetString("DoNotUseUsingEntityOnSharedClrType", nameof(clrType)), + clrType); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/EFCore/Properties/CoreStrings.resx b/src/EFCore/Properties/CoreStrings.resx index 6a666614dbd..5272506faf7 100644 --- a/src/EFCore/Properties/CoreStrings.resx +++ b/src/EFCore/Properties/CoreStrings.resx @@ -1421,10 +1421,16 @@ Unhandled 'INavigationBase' of type '{type}'. - - Cannot create an association entity type using skip navigations '{leftEntityType}.{leftSkipNavigationName}' which targets entity type '{leftTargetEntityType}', and '{rightEntityType}.{rightSkipNavigationName}' which targets '{rightTargetEntityType}'. They should target one another. + + Cannot use using skip navigation '{declaringEntityType}.{skipNavigationName}' to create an association entity type. Its inverse is not set. The inverse should be the other skip navigation, '{otherEntityType}.{otherSkipNavigationName}'. + + + Cannot use using skip navigation '{declaringEntityType}.{skipNavigationName}' to create an association entity type. Its inverse is '{inverseEntityType}.{inverseSkipNavigationName}', but it should be '{otherEntityType}.{otherSkipNavigationName}'. Cannot create a foreign key for skip navigation '{declaringEntityType}.{skipNavigationName}' on association entity type '{associationEntityType}'. Ensure '{declaringEntityType}' has a primary key and is not ignored in the model. + + Cannot use UsingEntity() passing type '{clrType}' because the model contains shared entity type(s) with same type. Use a type which uniquely defines an entity type. + \ No newline at end of file diff --git a/test/EFCore.Tests/ApiConsistencyTest.cs b/test/EFCore.Tests/ApiConsistencyTest.cs index ebe9b756675..83df6f58f2f 100644 --- a/test/EFCore.Tests/ApiConsistencyTest.cs +++ b/test/EFCore.Tests/ApiConsistencyTest.cs @@ -137,7 +137,8 @@ public override bool TryGetProviderOptionsDelegate(out Action new ManyToManyConvention(CreateDependencies()); + private ManyToManyAssociationEntityTypeConvention CreateManyToManyConvention() + => new ManyToManyAssociationEntityTypeConvention(CreateDependencies()); private ProviderConventionSetBuilderDependencies CreateDependencies() => InMemoryTestHelpers.Instance.CreateContextServices().GetRequiredService() diff --git a/test/EFCore.Tests/Metadata/Conventions/RelationshipDiscoveryConventionTest.cs b/test/EFCore.Tests/Metadata/Conventions/RelationshipDiscoveryConventionTest.cs index 27649854f5a..7d447bf0604 100644 --- a/test/EFCore.Tests/Metadata/Conventions/RelationshipDiscoveryConventionTest.cs +++ b/test/EFCore.Tests/Metadata/Conventions/RelationshipDiscoveryConventionTest.cs @@ -272,35 +272,6 @@ public void Many_to_many_bidirectional_is_not_discovered_if_relationship_should_ Assert.Empty(derivedManyToManySecond.Metadata.GetSkipNavigations()); } - [ConditionalFact] - public void Many_to_many_bidirectional_is_not_discovered_if_skip_navigations_already_created() - { - var modelBuilder = CreateInternalModeBuilder(); - var manyToManyFirst = modelBuilder.Entity(typeof(ManyToManyFirst), ConfigurationSource.Convention); - var manyToManySecond = modelBuilder.Entity(typeof(ManyToManySecond), ConfigurationSource.Convention); - - manyToManyFirst.PrimaryKey(new[] { nameof(ManyToManyFirst.Id) }, ConfigurationSource.Convention); - manyToManySecond.PrimaryKey(new[] { nameof(ManyToManySecond.Id) }, ConfigurationSource.Convention); - - var skipNavOnFirst = manyToManyFirst.HasSkipNavigation( - new MemberIdentity(typeof(ManyToManyFirst).GetProperty(nameof(ManyToManyFirst.ManyToManySeconds))), - manyToManySecond.Metadata, - ConfigurationSource.Convention); - var skipNavOnSecond = manyToManySecond.HasSkipNavigation( - new MemberIdentity(typeof(ManyToManySecond).GetProperty(nameof(ManyToManySecond.ManyToManyFirsts))), - manyToManyFirst.Metadata, - ConfigurationSource.Convention); - - RunConvention(manyToManyFirst); - - Assert.Empty(manyToManyFirst.Metadata.Model.GetEntityTypes() - .Where(et => et.IsAutomaticallyCreatedAssociationEntityType)); - - // the previously created skip navigations are unaffected - Assert.Same(skipNavOnFirst.Metadata, manyToManyFirst.Metadata.GetSkipNavigations().Single()); - Assert.Same(skipNavOnSecond.Metadata, manyToManySecond.Metadata.GetSkipNavigations().Single()); - } - [ConditionalFact] public void Many_to_many_bidirectional_is_not_discovered_if_missing_left_primary_key() { @@ -315,8 +286,6 @@ public void Many_to_many_bidirectional_is_not_discovered_if_missing_left_primary Assert.Empty(manyToManyFirst.Metadata.Model.GetEntityTypes() .Where(et => et.IsAutomaticallyCreatedAssociationEntityType)); - Assert.Empty(manyToManyFirst.Metadata.GetSkipNavigations()); - Assert.Empty(manyToManySecond.Metadata.GetSkipNavigations()); } [ConditionalFact] @@ -333,8 +302,6 @@ public void Many_to_many_bidirectional_is_not_discovered_if_missing_right_primar Assert.Empty(manyToManyFirst.Metadata.Model.GetEntityTypes() .Where(et => et.IsAutomaticallyCreatedAssociationEntityType)); - Assert.Empty(manyToManyFirst.Metadata.GetSkipNavigations()); - Assert.Empty(manyToManySecond.Metadata.GetSkipNavigations()); } [ConditionalFact] diff --git a/test/EFCore.Tests/Metadata/Internal/InternalModelBuilderTest.cs b/test/EFCore.Tests/Metadata/Internal/InternalModelBuilderTest.cs index 8748c4925e1..10acc05e49d 100644 --- a/test/EFCore.Tests/Metadata/Internal/InternalModelBuilderTest.cs +++ b/test/EFCore.Tests/Metadata/Internal/InternalModelBuilderTest.cs @@ -349,34 +349,70 @@ public void Can_mark_type_as_owned_type() } [ConditionalFact] - public void Throws_if_create_association_entity_type_with_invalid_left_skip_navigation() + public void Throws_if_create_association_entity_type_with_left_skip_navigation_with_no_inverse() { var model = new Model(); var modelBuilder = CreateModelBuilder(model); var manyToManyLeft = modelBuilder.Entity(typeof(ManyToManyLeft), ConfigurationSource.Convention); var manyToManyRight = modelBuilder.Entity(typeof(ManyToManyRight), ConfigurationSource.Convention); - var manyToManyOtherRight = modelBuilder.Entity(typeof(ManyToManyOtherRight), ConfigurationSource.Convention); - // skipNavOnLeft does not target skipNavOnRight.DeclaringEntityType var skipNavOnLeft = manyToManyLeft.HasSkipNavigation( - new MemberIdentity(typeof(ManyToManyLeft).GetProperty(nameof(ManyToManyLeft.OtherRights))), - manyToManyOtherRight.Metadata, + new MemberIdentity(typeof(ManyToManyLeft).GetProperty(nameof(ManyToManyLeft.Rights))), + manyToManyRight.Metadata, ConfigurationSource.Convention); var skipNavOnRight = manyToManyRight.HasSkipNavigation( new MemberIdentity(typeof(ManyToManyRight).GetProperty(nameof(ManyToManyRight.Lefts))), manyToManyLeft.Metadata, ConfigurationSource.Convention); + // set no inverse on skipNavOnLeft Assert.Equal( - CoreStrings.InvalidSkipNavigationsForAssociationEntityType( + CoreStrings.SkipNavigationForAssociationEntityTypeHasNoInverse( "Microsoft.EntityFrameworkCore.Metadata.Internal.InternalModelBuilderTest+ManyToManyLeft", - nameof(ManyToManyLeft.OtherRights), - "Microsoft.EntityFrameworkCore.Metadata.Internal.InternalModelBuilderTest+ManyToManyOtherRight", + nameof(ManyToManyLeft.Rights), + "Microsoft.EntityFrameworkCore.Metadata.Internal.InternalModelBuilderTest+ManyToManyRight", + nameof(ManyToManyRight.Lefts)), + Assert.Throws(() => modelBuilder.HasAutomaticAssociationEntity( + skipNavOnLeft.Metadata, skipNavOnRight.Metadata, ConfigurationSource.Explicit)).Message); + + Assert.Empty(model.GetEntityTypes() + .Where(e => e.IsAutomaticallyCreatedAssociationEntityType)); + } + + [ConditionalFact] + public void Throws_if_create_association_entity_type_with_right_skip_navigation_with_no_inverse() + { + var model = new Model(); + var modelBuilder = CreateModelBuilder(model); + + var manyToManyLeft = modelBuilder.Entity(typeof(ManyToManyLeft), ConfigurationSource.Convention); + var manyToManyRight = modelBuilder.Entity(typeof(ManyToManyRight), ConfigurationSource.Convention); + + var skipNavOnLeft = manyToManyLeft.HasSkipNavigation( + new MemberIdentity(typeof(ManyToManyLeft).GetProperty(nameof(ManyToManyLeft.Rights))), + manyToManyRight.Metadata, + ConfigurationSource.Convention); + var skipNavOnRight = manyToManyRight.HasSkipNavigation( + new MemberIdentity(typeof(ManyToManyRight).GetProperty(nameof(ManyToManyRight.Lefts))), + manyToManyLeft.Metadata, + ConfigurationSource.Convention); + var otherSkipNavOnRight = manyToManyRight.HasSkipNavigation( + new MemberIdentity(typeof(ManyToManyRight).GetProperty(nameof(ManyToManyRight.OtherLefts))), + manyToManyLeft.Metadata, + ConfigurationSource.Convention); + + // skipNavOnRight does not have an inverse - using SetInverse (rather than HasInverse) + // does _not_ set the inverse's inverse + skipNavOnLeft.Metadata.SetInverse(otherSkipNavOnRight.Metadata, ConfigurationSource.Convention); + + Assert.Equal( + CoreStrings.SkipNavigationForAssociationEntityTypeHasNoInverse( "Microsoft.EntityFrameworkCore.Metadata.Internal.InternalModelBuilderTest+ManyToManyRight", nameof(ManyToManyRight.Lefts), - "Microsoft.EntityFrameworkCore.Metadata.Internal.InternalModelBuilderTest+ManyToManyLeft"), - Assert.Throws(() => modelBuilder.AssociationEntity( + "Microsoft.EntityFrameworkCore.Metadata.Internal.InternalModelBuilderTest+ManyToManyLeft", + nameof(ManyToManyLeft.Rights)), + Assert.Throws(() => modelBuilder.HasAutomaticAssociationEntity( skipNavOnLeft.Metadata, skipNavOnRight.Metadata, ConfigurationSource.Explicit)).Message); Assert.Empty(model.GetEntityTypes() @@ -384,34 +420,83 @@ public void Throws_if_create_association_entity_type_with_invalid_left_skip_navi } [ConditionalFact] - public void Throws_if_create_association_entity_type_with_invalid_right_skip_navigation() + public void Throws_if_create_association_entity_type_with_left_skip_navigation_with_wrong_inverse() { var model = new Model(); var modelBuilder = CreateModelBuilder(model); var manyToManyLeft = modelBuilder.Entity(typeof(ManyToManyLeft), ConfigurationSource.Convention); var manyToManyRight = modelBuilder.Entity(typeof(ManyToManyRight), ConfigurationSource.Convention); - var manyToManyOtherLeft = modelBuilder.Entity(typeof(ManyToManyOtherLeft), ConfigurationSource.Convention); - // skipNavOnRight does not target skipNavOnLeft.DeclaringEntityType var skipNavOnLeft = manyToManyLeft.HasSkipNavigation( new MemberIdentity(typeof(ManyToManyLeft).GetProperty(nameof(ManyToManyLeft.Rights))), manyToManyRight.Metadata, ConfigurationSource.Convention); var skipNavOnRight = manyToManyRight.HasSkipNavigation( + new MemberIdentity(typeof(ManyToManyRight).GetProperty(nameof(ManyToManyRight.Lefts))), + manyToManyLeft.Metadata, + ConfigurationSource.Convention); + var otherSkipNavOnRight = manyToManyRight.HasSkipNavigation( new MemberIdentity(typeof(ManyToManyRight).GetProperty(nameof(ManyToManyRight.OtherLefts))), - manyToManyOtherLeft.Metadata, + manyToManyLeft.Metadata, ConfigurationSource.Convention); + // using SetInverse (rather than HasInverse) does _not_ set the inverse's inverse + skipNavOnRight.Metadata.SetInverse(skipNavOnLeft.Metadata, ConfigurationSource.Convention); // correct + skipNavOnLeft.Metadata.SetInverse(otherSkipNavOnRight.Metadata, ConfigurationSource.Convention); // wrong + Assert.Equal( - CoreStrings.InvalidSkipNavigationsForAssociationEntityType( + CoreStrings.SkipNavigationForAssociationEntityTypeWrongInverse( "Microsoft.EntityFrameworkCore.Metadata.Internal.InternalModelBuilderTest+ManyToManyLeft", nameof(ManyToManyLeft.Rights), "Microsoft.EntityFrameworkCore.Metadata.Internal.InternalModelBuilderTest+ManyToManyRight", - "Microsoft.EntityFrameworkCore.Metadata.Internal.InternalModelBuilderTest+ManyToManyRight", nameof(ManyToManyRight.OtherLefts), - "Microsoft.EntityFrameworkCore.Metadata.Internal.InternalModelBuilderTest+ManyToManyOtherLeft"), - Assert.Throws(() => modelBuilder.AssociationEntity( + "Microsoft.EntityFrameworkCore.Metadata.Internal.InternalModelBuilderTest+ManyToManyRight", + nameof(ManyToManyRight.Lefts) + ), + Assert.Throws(() => modelBuilder.HasAutomaticAssociationEntity( + skipNavOnLeft.Metadata, skipNavOnRight.Metadata, ConfigurationSource.Explicit)).Message); + + Assert.Empty(model.GetEntityTypes() + .Where(e => e.IsAutomaticallyCreatedAssociationEntityType)); + } + + [ConditionalFact] + public void Throws_if_create_association_entity_type_with_right_skip_navigation_with_wrong_inverse() + { + var model = new Model(); + var modelBuilder = CreateModelBuilder(model); + + var manyToManyLeft = modelBuilder.Entity(typeof(ManyToManyLeft), ConfigurationSource.Convention); + var manyToManyRight = modelBuilder.Entity(typeof(ManyToManyRight), ConfigurationSource.Convention); + + var skipNavOnLeft = manyToManyLeft.HasSkipNavigation( + new MemberIdentity(typeof(ManyToManyLeft).GetProperty(nameof(ManyToManyLeft.Rights))), + manyToManyRight.Metadata, + ConfigurationSource.Convention); + var skipNavOnRight = manyToManyRight.HasSkipNavigation( + new MemberIdentity(typeof(ManyToManyRight).GetProperty(nameof(ManyToManyRight.Lefts))), + manyToManyLeft.Metadata, + ConfigurationSource.Convention); + var otherSkipNavOnLeft = manyToManyLeft.HasSkipNavigation( + new MemberIdentity(typeof(ManyToManyLeft).GetProperty(nameof(ManyToManyLeft.OtherRights))), + manyToManyRight.Metadata, + ConfigurationSource.Convention); + + // using SetInverse (rather than HasInverse) does _not_ set the inverse's inverse + skipNavOnLeft.Metadata.SetInverse(skipNavOnRight.Metadata, ConfigurationSource.Convention); // correct + skipNavOnRight.Metadata.SetInverse(otherSkipNavOnLeft.Metadata, ConfigurationSource.Convention); // wrong + + Assert.Equal( + CoreStrings.SkipNavigationForAssociationEntityTypeWrongInverse( + "Microsoft.EntityFrameworkCore.Metadata.Internal.InternalModelBuilderTest+ManyToManyRight", + nameof(ManyToManyRight.Lefts), + "Microsoft.EntityFrameworkCore.Metadata.Internal.InternalModelBuilderTest+ManyToManyLeft", + nameof(ManyToManyLeft.OtherRights), + "Microsoft.EntityFrameworkCore.Metadata.Internal.InternalModelBuilderTest+ManyToManyLeft", + nameof(ManyToManyLeft.Rights) + ), + Assert.Throws(() => modelBuilder.HasAutomaticAssociationEntity( skipNavOnLeft.Metadata, skipNavOnRight.Metadata, ConfigurationSource.Explicit)).Message); Assert.Empty(model.GetEntityTypes() @@ -437,13 +522,14 @@ public void Throws_if_create_association_entity_type_with_left_entity_type_with_ new MemberIdentity(typeof(ManyToManyRight).GetProperty(nameof(ManyToManyRight.Lefts))), manyToManyLeft.Metadata, ConfigurationSource.Convention); + skipNavOnLeft.HasInverse(skipNavOnRight.Metadata, ConfigurationSource.Convention); Assert.Equal( CoreStrings.UnableToCreateSkipNavigationForeignKeyOnAssociationEntityType( "Microsoft.EntityFrameworkCore.Metadata.Internal.InternalModelBuilderTest+ManyToManyLeft", nameof(ManyToManyLeft.Rights), "Join_ManyToManyLeft_ManyToManyRight"), - Assert.Throws(() => modelBuilder.AssociationEntity( + Assert.Throws(() => modelBuilder.HasAutomaticAssociationEntity( skipNavOnLeft.Metadata, skipNavOnRight.Metadata, ConfigurationSource.Explicit)).Message); Assert.Empty(model.GetEntityTypes() @@ -469,13 +555,14 @@ public void Throws_if_create_association_entity_type_with_right_entity_type_with new MemberIdentity(typeof(ManyToManyRight).GetProperty(nameof(ManyToManyRight.Lefts))), manyToManyLeft.Metadata, ConfigurationSource.Convention); + skipNavOnLeft.HasInverse(skipNavOnRight.Metadata, ConfigurationSource.Convention); Assert.Equal( CoreStrings.UnableToCreateSkipNavigationForeignKeyOnAssociationEntityType( "Microsoft.EntityFrameworkCore.Metadata.Internal.InternalModelBuilderTest+ManyToManyRight", nameof(ManyToManyRight.Lefts), "Join_ManyToManyLeft_ManyToManyRight"), - Assert.Throws(() => modelBuilder.AssociationEntity( + Assert.Throws(() => modelBuilder.HasAutomaticAssociationEntity( skipNavOnLeft.Metadata, skipNavOnRight.Metadata, ConfigurationSource.Explicit)).Message); Assert.Empty(model.GetEntityTypes() @@ -501,8 +588,9 @@ public void Can_create_association_entity_type() new MemberIdentity(typeof(ManyToManyRight).GetProperty(nameof(ManyToManyRight.Lefts))), manyToManyLeft.Metadata, ConfigurationSource.Convention); + skipNavOnLeft.HasInverse(skipNavOnRight.Metadata, ConfigurationSource.Convention); - var associationEntityTypeBuilder = modelBuilder.AssociationEntity( + var associationEntityTypeBuilder = modelBuilder.HasAutomaticAssociationEntity( skipNavOnLeft.Metadata, skipNavOnRight.Metadata, ConfigurationSource.Convention); var associationEntityType = associationEntityTypeBuilder.Metadata; @@ -559,8 +647,9 @@ public void Can_remove_automatically_created_association_entity_type(bool remove new MemberIdentity(typeof(ManyToManyRight).GetProperty(nameof(ManyToManyRight.Lefts))), manyToManyLeft.Metadata, ConfigurationSource.Convention); + skipNavOnLeft.HasInverse(skipNavOnRight.Metadata, ConfigurationSource.Convention); - var associationEntityTypeBuilder = modelBuilder.AssociationEntity( + var associationEntityTypeBuilder = modelBuilder.HasAutomaticAssociationEntity( skipNavOnLeft.Metadata, skipNavOnRight.Metadata, ConfigurationSource.Convention); var associationEntityType = associationEntityTypeBuilder.Metadata; @@ -609,6 +698,7 @@ public void Cannot_remove_manually_created_association_entity_type(bool removeSk new MemberIdentity(typeof(ManyToManyRight).GetProperty(nameof(ManyToManyRight.Lefts))), manyToManyLeft.Metadata, ConfigurationSource.Convention); + skipNavOnLeft.HasInverse(skipNavOnRight.Metadata, ConfigurationSource.Convention); var leftFK = manyToManyJoin.HasRelationship( manyToManyLeft.Metadata.Name, @@ -702,26 +792,14 @@ private class ManyToManyLeft { public int Id { get; set; } public List Rights { get; set; } - public List OtherRights { get; set; } + public List OtherRights { get; set; } } private class ManyToManyRight { public int Id { get; set; } public List Lefts { get; set; } - public List OtherLefts { get; set; } - } - - private class ManyToManyOtherLeft - { - public int Id { get; set; } - public List Rights { get; set; } - } - - private class ManyToManyOtherRight - { - public int Id { get; set; } - public List Lefts { get; set; } + public List OtherLefts { get; set; } } private class ManyToManyJoin