diff --git a/src/EFCore/Metadata/Builders/CollectionCollectionBuilder.cs b/src/EFCore/Metadata/Builders/CollectionCollectionBuilder.cs index 2131ba12e1d..3067d5fb342 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,6 +95,23 @@ public virtual EntityTypeBuilder UsingEntity( [NotNull] Func configureRight, [NotNull] Func configureLeft) { + if (((Model)LeftEntityType.Model).IsShared(joinEntity)) + { + //TODO #9914 - when the generic version of "please use the shared-type entity type version of this API" + // is available then update to use that. + throw new InvalidOperationException( + CoreStrings.DoNotUseUsingEntityOnSharedClrType(joinEntity.GetType().Name)); + } + + var existingAssociationEntityType = (EntityType) + (LeftNavigation.ForeignKey?.DeclaringEntityType + ?? RightNavigation.ForeignKey?.DeclaringEntityType); + if (existingAssociationEntityType != null) + { + ModelBuilder.RemoveAssociationEntityIfCreatedImplicitly( + existingAssociationEntityType, removeSkipNavigations: false, ConfigurationSource.Explicit); + } + var entityTypeBuilder = new EntityTypeBuilder( ModelBuilder.Entity(joinEntity, ConfigurationSource.Explicit).Metadata); diff --git a/src/EFCore/Metadata/Builders/CollectionCollectionBuilder`.cs b/src/EFCore/Metadata/Builders/CollectionCollectionBuilder`.cs index 90758064e0a..1449196dc76 100644 --- a/src/EFCore/Metadata/Builders/CollectionCollectionBuilder`.cs +++ b/src/EFCore/Metadata/Builders/CollectionCollectionBuilder`.cs @@ -3,7 +3,9 @@ using System; using JetBrains.Annotations; +using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Metadata.Internal; namespace Microsoft.EntityFrameworkCore.Metadata.Builders { @@ -50,6 +52,23 @@ public virtual EntityTypeBuilder UsingEntity, ReferenceCollectionBuilder> configureLeft) where TAssociationEntity : class { + if (((Model)LeftEntityType.Model).IsShared(typeof(TAssociationEntity))) + { + //TODO #9914 - when the generic version of "please use the shared-type entity type version of this API" + // is available then update to use that. + throw new InvalidOperationException( + CoreStrings.DoNotUseUsingEntityOnSharedClrType(typeof(TAssociationEntity).Name)); + } + + var existingAssociationEntityType = (EntityType) + (LeftNavigation.ForeignKey?.DeclaringEntityType + ?? RightNavigation.ForeignKey?.DeclaringEntityType); + if (existingAssociationEntityType != null) + { + ModelBuilder.RemoveAssociationEntityIfCreatedImplicitly( + existingAssociationEntityType, removeSkipNavigations: false, ConfigurationSource.Explicit); + } + var entityTypeBuilder = new EntityTypeBuilder( ModelBuilder.Entity(typeof(TAssociationEntity), ConfigurationSource.Explicit).Metadata); diff --git a/src/EFCore/Metadata/Builders/CollectionNavigationBuilder.cs b/src/EFCore/Metadata/Builders/CollectionNavigationBuilder.cs index 7c36f3f3a0c..f0c396da96a 100644 --- a/src/EFCore/Metadata/Builders/CollectionNavigationBuilder.cs +++ b/src/EFCore/Metadata/Builders/CollectionNavigationBuilder.cs @@ -151,16 +151,38 @@ private InternalForeignKeyBuilder WithOneBuilder(MemberIdentity reference) { if (SkipNavigation != null) { - throw new InvalidOperationException( - CoreStrings.ConflictingRelationshipNavigation( - SkipNavigation.DeclaringEntityType.DisplayName() + "." + SkipNavigation.Name, - RelatedEntityType.DisplayName() + (reference.Name == null - ? "" - : "." + reference.Name), - SkipNavigation.DeclaringEntityType.DisplayName() + "." + SkipNavigation.Name, - SkipNavigation.TargetEntityType.DisplayName() + (SkipNavigation.Inverse == null - ? "" - : "." + SkipNavigation.Inverse.Name))); + // Note: we delayed setting the ConfigurationSource of SkipNavigation in HasMany() + // so we can test it here and override if the skip navigation was originally found + // by convention. + if (((IConventionSkipNavigation)SkipNavigation).GetConfigurationSource() == ConfigurationSource.Explicit) + { + throw new InvalidOperationException( + CoreStrings.ConflictingRelationshipNavigation( + SkipNavigation.DeclaringEntityType.DisplayName() + "." + SkipNavigation.Name, + RelatedEntityType.DisplayName() + (reference.Name == null + ? "" + : "." + reference.Name), + SkipNavigation.DeclaringEntityType.DisplayName() + "." + SkipNavigation.Name, + SkipNavigation.TargetEntityType.DisplayName() + (SkipNavigation.Inverse == null + ? "" + : "." + SkipNavigation.Inverse.Name))); + } + + var navigationName = SkipNavigation.Name; + var declaringEntityType = (EntityType)DeclaringEntityType; + declaringEntityType.Model.Builder + .RemoveAssociationEntityIfCreatedImplicitly( + (EntityType)SkipNavigation.AssociationEntityType, + removeSkipNavigations: true, + ConfigurationSource.Explicit); + + Builder = declaringEntityType.Builder + .HasRelationship( + (EntityType)RelatedEntityType, + navigationName, + ConfigurationSource.Explicit, + targetIsPrincipal: false); + SkipNavigation = null; } var foreignKey = Builder.Metadata; @@ -205,11 +227,39 @@ 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)); + + Configure(collectionCollectionBuilder); + + return collectionCollectionBuilder; + } + + /// + /// 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. + /// + [EntityFrameworkInternal] + protected virtual void Configure([NotNull] CollectionCollectionBuilder collectionCollectionBuilder) + { + Check.NotNull(collectionCollectionBuilder, nameof(collectionCollectionBuilder)); + + var leftSkipNavigation = (SkipNavigation)collectionCollectionBuilder.LeftNavigation; + var rightSkipNavigation = (SkipNavigation)collectionCollectionBuilder.RightNavigation; + + leftSkipNavigation.Builder.HasInverse(rightSkipNavigation, ConfigurationSource.Explicit); + + // Note: we delayed setting the ConfigurationSource of SkipNavigation + // in HasMany(). But now we know that both skip navigations should + // have ConfigurationSource.Explicit. + leftSkipNavigation.UpdateConfigurationSource(ConfigurationSource.Explicit); + rightSkipNavigation.UpdateConfigurationSource(ConfigurationSource.Explicit); } /// diff --git a/src/EFCore/Metadata/Builders/CollectionNavigationBuilder`.cs b/src/EFCore/Metadata/Builders/CollectionNavigationBuilder`.cs index c5abc27f5a6..f3134e5ef46 100644 --- a/src/EFCore/Metadata/Builders/CollectionNavigationBuilder`.cs +++ b/src/EFCore/Metadata/Builders/CollectionNavigationBuilder`.cs @@ -95,11 +95,16 @@ 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)); + + Configure(collectionCollectionBuilder); + + return collectionCollectionBuilder; } /// @@ -126,11 +131,16 @@ 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)); + + Configure(collectionCollectionBuilder); + + return collectionCollectionBuilder; } } } diff --git a/src/EFCore/Metadata/Builders/EntityTypeBuilder.cs b/src/EFCore/Metadata/Builders/EntityTypeBuilder.cs index a80c5056a06..7ccb3d02088 100644 --- a/src/EFCore/Metadata/Builders/EntityTypeBuilder.cs +++ b/src/EFCore/Metadata/Builders/EntityTypeBuilder.cs @@ -832,6 +832,10 @@ private CollectionNavigationBuilder HasMany( string navigationName, EntityType relatedEntityType) { + // Note: delay setting ConfigurationSource of skip navigation (if it exists). + // We do not yet know whether this will be a HasMany().WithOne() or a + // HasMany().WithMany(). If the skip navigation was found by convention + // we want to be able to override it later. var skipNavigation = navigationName != null ? Builder.Metadata.FindSkipNavigation(navigationName) : null; InternalForeignKeyBuilder relationship = null; diff --git a/src/EFCore/Metadata/Builders/EntityTypeBuilder`.cs b/src/EFCore/Metadata/Builders/EntityTypeBuilder`.cs index 691b7750612..d5a00ae7610 100644 --- a/src/EFCore/Metadata/Builders/EntityTypeBuilder`.cs +++ b/src/EFCore/Metadata/Builders/EntityTypeBuilder`.cs @@ -702,6 +702,11 @@ public virtual CollectionNavigationBuilder HasMany HasMany Additional information associated with convention execution. void ProcessSkipNavigationInverseChanged( [NotNull] IConventionSkipNavigationBuilder skipNavigationBuilder, - [NotNull] IConventionSkipNavigation inverse, - [NotNull] IConventionSkipNavigation oldInverse, + [CanBeNull] IConventionSkipNavigation inverse, + [CanBeNull] IConventionSkipNavigation oldInverse, [NotNull] IConventionContext context); } } diff --git a/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs b/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs index 919fcf3d5ad..9ad841f3aaf 100644 --- a/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs +++ b/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs @@ -200,6 +200,8 @@ public virtual ConventionSet CreateConventionSet() conventionSet.NavigationRemovedConventions.Add(relationshipDiscoveryConvention); + conventionSet.SkipNavigationAddedConventions.Add(new ManyToManyAssociationEntityTypeConvention(Dependencies)); + conventionSet.IndexAddedConventions.Add(foreignKeyIndexConvention); conventionSet.IndexRemovedConventions.Add(foreignKeyIndexConvention); diff --git a/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.ImmediateConventionScope.cs b/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.ImmediateConventionScope.cs index be8a37b18c8..d25d5e505e3 100644 --- a/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.ImmediateConventionScope.cs +++ b/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.ImmediateConventionScope.cs @@ -710,7 +710,6 @@ public override IConventionSkipNavigationBuilder OnSkipNavigationAdded( { if (navigationBuilder.Metadata.Builder == null) { - Check.DebugAssert(false, "null builder"); return null; } diff --git a/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.cs b/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.cs index d0a198fb4d7..4bec126cd58 100644 --- a/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.cs +++ b/src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.cs @@ -352,8 +352,8 @@ public virtual IConventionForeignKey OnSkipNavigationForeignKeyChanged( /// public virtual IConventionSkipNavigation OnSkipNavigationInverseChanged( [NotNull] IConventionSkipNavigationBuilder navigationBuilder, - [NotNull] IConventionSkipNavigation inverse, - [NotNull] IConventionSkipNavigation oldInverse) + [CanBeNull] IConventionSkipNavigation inverse, + [CanBeNull] IConventionSkipNavigation oldInverse) => _scope.OnSkipNavigationInverseChanged(navigationBuilder, inverse, oldInverse); /// diff --git a/src/EFCore/Metadata/Conventions/ManyToManyAssociationEntityTypeConvention.cs b/src/EFCore/Metadata/Conventions/ManyToManyAssociationEntityTypeConvention.cs new file mode 100644 index 00000000000..135e6b32ee6 --- /dev/null +++ b/src/EFCore/Metadata/Conventions/ManyToManyAssociationEntityTypeConvention.cs @@ -0,0 +1,186 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Generic; +using System.Linq; +using JetBrains.Annotations; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Metadata.Builders; +using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure; +using Microsoft.EntityFrameworkCore.Metadata.Internal; +using Microsoft.EntityFrameworkCore.Utilities; + +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. + /// + public class ManyToManyAssociationEntityTypeConvention : ISkipNavigationAddedConvention, ISkipNavigationInverseChangedConvention + { + private const string AssociationEntityTypeNameTemplate = "Join_{0}_{1}"; + private const string AssociationPropertyNameTemplate = "{0}_{1}"; + + /// + /// Creates a new instance of . + /// + /// Parameter object containing dependencies for this convention. + public ManyToManyAssociationEntityTypeConvention([NotNull] ProviderConventionSetBuilderDependencies dependencies) + { + Dependencies = dependencies; + } + + /// + /// Parameter object containing service dependencies. + /// + protected virtual ProviderConventionSetBuilderDependencies Dependencies { get; } + + /// + public virtual void ProcessSkipNavigationAdded( + IConventionSkipNavigationBuilder skipNavigationBuilder, + IConventionContext context) + { + Check.NotNull(skipNavigationBuilder, nameof(skipNavigationBuilder)); + Check.NotNull(context, nameof(context)); + + CreateAssociationEntityType(skipNavigationBuilder); + } + + /// + public virtual void ProcessSkipNavigationInverseChanged( + IConventionSkipNavigationBuilder skipNavigationBuilder, + IConventionSkipNavigation inverse, + IConventionSkipNavigation oldInverse, + IConventionContext context) + { + Check.NotNull(skipNavigationBuilder, nameof(skipNavigationBuilder)); + Check.NotNull(context, nameof(context)); + + CreateAssociationEntityType(skipNavigationBuilder); + } + + private void CreateAssociationEntityType( + IConventionSkipNavigationBuilder skipNavigationBuilder) + { + var skipNavigation = (SkipNavigation)skipNavigationBuilder.Metadata; + if (skipNavigation.AssociationEntityType != null) + { + return; + } + + if (skipNavigation.ForeignKey != null + || skipNavigation.TargetEntityType == skipNavigation.DeclaringEntityType + || !skipNavigation.IsCollection) + { + // do not create the association entity type for a self-referencing + // skip navigation, or for one that is already "in use" + // (i.e. has its Foreign Key assigned). + return; + } + + var inverseSkipNavigation = skipNavigation.Inverse; + if (inverseSkipNavigation == null + || inverseSkipNavigation.ForeignKey != null + || !inverseSkipNavigation.IsCollection) + { + // do not create the association entity type if + // the inverse skip navigation is already "in use" + // (i.e. has its Foreign Key assigned). + return; + } + + Check.DebugAssert(inverseSkipNavigation.Inverse == skipNavigation, + "Inverse's inverse should be the original skip navigation"); + + var declaringEntityType = skipNavigation.DeclaringEntityType; + var inverseEntityType = inverseSkipNavigation.DeclaringEntityType; + var model = declaringEntityType.Model; + + // create the association entity type + var otherIdentifiers = model.GetEntityTypes().ToDictionary(et => et.Name, et => 0); + var associationEntityTypeName = Uniquifier.Uniquify( + string.Format( + AssociationEntityTypeNameTemplate, + declaringEntityType.ShortName(), + inverseEntityType.ShortName()), + otherIdentifiers, + int.MaxValue); + //TODO #9914 - when the shared-type entity type version of model.Entity() is available call that instead + var associationEntityTypeBuilder = + model.AddEntityType( + associationEntityTypeName, + Model.DefaultPropertyBagType, + ConfigurationSource.Convention).Builder; + + // Create left and right foreign keys from the outer entity types to + // the association entity type and configure the skip navigations. + // Roll back if any of this fails. + var leftForeignKey = + CreateSkipNavigationForeignKey(skipNavigation, associationEntityTypeBuilder); + if (leftForeignKey == null) + { + model.Builder.HasNoEntityType( + associationEntityTypeBuilder.Metadata, ConfigurationSource.Convention); + return; + } + + var rightForeignKey = + CreateSkipNavigationForeignKey(inverseSkipNavigation, associationEntityTypeBuilder); + if (rightForeignKey == null) + { + // Removing the association entity type will also remove + // the leftForeignKey created above. + model.Builder.HasNoEntityType( + associationEntityTypeBuilder.Metadata, ConfigurationSource.Convention); + return; + } + + skipNavigation.Builder.HasForeignKey(leftForeignKey, ConfigurationSource.Convention); + inverseSkipNavigation.Builder.HasForeignKey(rightForeignKey, ConfigurationSource.Convention); + + // Creating the primary key below also negates the need for an index on + // the properties of leftForeignKey - that index is automatically removed. + associationEntityTypeBuilder.PrimaryKey( + leftForeignKey.Properties.Concat(rightForeignKey.Properties).ToList(), + ConfigurationSource.Convention); + } + + private static ForeignKey CreateSkipNavigationForeignKey( + SkipNavigation skipNavigation, + InternalEntityTypeBuilder associationEntityTypeBuilder) + { + var principalEntityType = skipNavigation.DeclaringEntityType; + var principalKey = principalEntityType.FindPrimaryKey(); + if (principalKey == null) + { + return null; + } + + var dependentEndForeignKeyPropertyNames = new List(); + var otherIdentifiers = associationEntityTypeBuilder.Metadata + .GetDeclaredProperties().ToDictionary(p => p.Name, p => 0); + foreach (var property in principalKey.Properties) + { + var propertyName = Uniquifier.Uniquify( + string.Format( + AssociationPropertyNameTemplate, + principalEntityType.ShortName(), + property.Name), + otherIdentifiers, + int.MaxValue); + dependentEndForeignKeyPropertyNames.Add(propertyName); + otherIdentifiers.Add(propertyName, 0); + } + + return associationEntityTypeBuilder + .HasRelationship( + principalEntityType.Name, + dependentEndForeignKeyPropertyNames, + principalKey, + ConfigurationSource.Convention) + .IsUnique(false, ConfigurationSource.Convention) + .Metadata; + } + } +} diff --git a/src/EFCore/Metadata/Conventions/ModelCleanupConvention.cs b/src/EFCore/Metadata/Conventions/ModelCleanupConvention.cs index d2372e891ff..6a6f2d9e0fc 100644 --- a/src/EFCore/Metadata/Conventions/ModelCleanupConvention.cs +++ b/src/EFCore/Metadata/Conventions/ModelCleanupConvention.cs @@ -71,7 +71,8 @@ private IReadOnlyList GetRoots(IConventionModel model, Co private void RemoveNavigationlessForeignKeys(IConventionModelBuilder modelBuilder) { - foreach (var entityType in modelBuilder.Metadata.GetEntityTypes()) + foreach (var entityType in modelBuilder.Metadata.GetEntityTypes() + .Where(e => !((EntityType)e).IsImplicitlyCreatedAssociationEntityType)) { foreach (var foreignKey in entityType.GetDeclaredForeignKeys().ToList()) { @@ -106,11 +107,15 @@ public GraphAdapter([NotNull] IConventionModel model) public override IEnumerable GetOutgoingNeighbors(IConventionEntityType from) => from.GetForeignKeys().Where(fk => fk.DependentToPrincipal != null).Select(fk => fk.PrincipalEntityType) - .Union(from.GetReferencingForeignKeys().Where(fk => fk.PrincipalToDependent != null).Select(fk => fk.DeclaringEntityType)); + .Union(from.GetReferencingForeignKeys().Where(fk => fk.PrincipalToDependent != null).Select(fk => fk.DeclaringEntityType)) + .Union(from.GetSkipNavigations().Where(sn => sn.ForeignKey != null).Select(sn => sn.ForeignKey.DeclaringEntityType)) + .Union(from.GetSkipNavigations().Where(sn => sn.TargetEntityType != null).Select(sn => sn.TargetEntityType)); public override IEnumerable GetIncomingNeighbors(IConventionEntityType to) => to.GetForeignKeys().Where(fk => fk.PrincipalToDependent != null).Select(fk => fk.PrincipalEntityType) - .Union(to.GetReferencingForeignKeys().Where(fk => fk.DependentToPrincipal != null).Select(fk => fk.DeclaringEntityType)); + .Union(to.GetReferencingForeignKeys().Where(fk => fk.DependentToPrincipal != null).Select(fk => fk.DeclaringEntityType)) + .Union(to.GetSkipNavigations().Where(sn => sn.ForeignKey != null).Select(sn => sn.ForeignKey.DeclaringEntityType)) + .Union(to.GetSkipNavigations().Where(sn => sn.TargetEntityType != null).Select(sn => sn.TargetEntityType)); public override void Clear() { diff --git a/src/EFCore/Metadata/Conventions/RelationshipDiscoveryConvention.cs b/src/EFCore/Metadata/Conventions/RelationshipDiscoveryConvention.cs index 4235aaf17fa..0a48f6fd6df 100644 --- a/src/EFCore/Metadata/Conventions/RelationshipDiscoveryConvention.cs +++ b/src/EFCore/Metadata/Conventions/RelationshipDiscoveryConvention.cs @@ -239,13 +239,27 @@ private static IReadOnlyList RemoveIncompatibleWithExisti while (relationshipCandidate.NavigationProperties.Count > 0) { var navigationProperty = relationshipCandidate.NavigationProperties[0]; - var existingNavigation = entityType.FindNavigation(navigationProperty.GetSimpleMemberName()); - if (existingNavigation != null - && (existingNavigation.DeclaringEntityType != entityType - || existingNavigation.TargetEntityType != targetEntityType)) + var navigationPropertyName = navigationProperty.GetSimpleMemberName(); + var existingNavigation = entityType.FindNavigation(navigationPropertyName); + if (existingNavigation != null) { - relationshipCandidate.NavigationProperties.Remove(navigationProperty); - continue; + if (existingNavigation.DeclaringEntityType != entityType + || existingNavigation.TargetEntityType != targetEntityType) + { + relationshipCandidate.NavigationProperties.Remove(navigationProperty); + continue; + } + } + else + { + var existingSkipNavigation = entityType.FindSkipNavigation(navigationPropertyName); + if (existingSkipNavigation != null + && (existingSkipNavigation.DeclaringEntityType != entityType + || existingSkipNavigation.TargetEntityType != targetEntityType)) + { + relationshipCandidate.NavigationProperties.Remove(navigationProperty); + continue; + } } if (relationshipCandidate.NavigationProperties.Count == 1 @@ -573,30 +587,14 @@ private void CreateRelationships( foreach (var navigationProperty in relationshipCandidate.NavigationProperties.ToList()) { - var existingNavigation = entityType.FindDeclaredNavigation(navigationProperty.GetSimpleMemberName()); - if (existingNavigation != null - && 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); - } + RemoveNavigation( + navigationProperty, entityType, relationshipCandidate.NavigationProperties); } foreach (var inverseProperty in relationshipCandidate.InverseProperties.ToList()) { - var existingInverse = targetEntityType.FindDeclaredNavigation(inverseProperty.GetSimpleMemberName()); - if (existingInverse != null - && 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); - } + RemoveNavigation( + inverseProperty, targetEntityType, relationshipCandidate.InverseProperties); } if (!isAmbiguousOnBase) @@ -677,10 +675,14 @@ private void CreateRelationships( } else { - entityTypeBuilder.HasRelationship( - targetEntityType, - navigation, - inverse); + if (entityTypeBuilder.HasRelationship( + targetEntityType, + navigation, + inverse) == null) + { + HasManyToManyRelationship( + entityTypeBuilder, relationshipCandidate, navigation, inverse); + } } } } @@ -725,6 +727,112 @@ private void CreateRelationships( } } + private void RemoveNavigation( + 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 implicitly created. + if (modelBuilder.RemoveAssociationEntityIfCreatedImplicitly( + associationEntityType, removeSkipNavigations: true, ConfigurationSource.Convention) == null) + { + // Navigations of higher configuration source are not ambiguous + toRemoveFrom.Remove(navigationProperty); + } + } + } + } + + private void HasManyToManyRelationship( + IConventionEntityTypeBuilder entityTypeBuilder, + RelationshipCandidate relationshipCandidate, + PropertyInfo navigation, + PropertyInfo inverse) + { + var navigationTargetType = navigation.PropertyType.TryGetSequenceType(); + var inverseTargetType = inverse.PropertyType.TryGetSequenceType(); + if (navigationTargetType == null + || inverseTargetType == null) + { + // these are not many-to-many navigations + return; + } + + if (navigationTargetType == inverseTargetType) + { + // do not automatically create many-to-many associations to self + return; + } + + var leftEntityType = entityTypeBuilder.Metadata; + var rightEntityType = relationshipCandidate.TargetTypeBuilder.Metadata; + if (navigationTargetType != rightEntityType.ClrType + || inverseTargetType != leftEntityType.ClrType) + { + // this relationship should be defined between different + // entity types further up at least one of their inheritance trees + return; + } + + var navigationName = navigation.GetSimpleMemberName(); + var inverseName = inverse.GetSimpleMemberName(); + if (((EntityType)leftEntityType) + .FindNavigationsInHierarchy(navigationName).FirstOrDefault() != null + || ((EntityType)rightEntityType) + .FindNavigationsInHierarchy(inverseName).FirstOrDefault() != null) + { + // navigations have already been configured using these properties + return; + } + + var leftSkipNavigation = leftEntityType.FindSkipNavigation(navigation) ?? + leftEntityType.AddSkipNavigation( + navigationName, navigation, rightEntityType, + collection: true, onDependent: false, fromDataAnnotation: false); + if (leftSkipNavigation == null) + { + return; + } + + var rightSkipNavigation = rightEntityType.FindSkipNavigation(inverse) ?? + rightEntityType.AddSkipNavigation( + inverseName, inverse, leftEntityType, + collection: true, onDependent: false, fromDataAnnotation: false); + if (rightSkipNavigation == null) + { + return; + } + + // also sets rightSkipNavigation's inverse + leftSkipNavigation.Builder.HasInverse(rightSkipNavigation, fromDataAnnotation: false); + + // the implicit many-to-many association entity type will be created + // and configured by ManyToManyAssocationEntityTypeConvention. + } + /// /// Called after an entity type is added to the model. /// 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/EntityType.cs b/src/EFCore/Metadata/Internal/EntityType.cs index 1f55aa40ace..f4ce7c392fe 100644 --- a/src/EFCore/Metadata/Internal/EntityType.cs +++ b/src/EFCore/Metadata/Internal/EntityType.cs @@ -3049,6 +3049,16 @@ public virtual void CheckDiscriminatorValue([NotNull] IEntityType entityType, [C } } + /// + /// 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 IsImplicitlyCreatedAssociationEntityType + => GetConfigurationSource() == ConfigurationSource.Convention + && ClrType == Model.DefaultPropertyBagType; + #endregion #region Explicit interface implementations diff --git a/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs b/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs index 06b7ee86f98..1882acc1b65 100644 --- a/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs +++ b/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs @@ -1050,6 +1050,7 @@ public virtual InternalEntityTypeBuilder Ignore([NotNull] string name, Configura && inverse.DeclaringEntityType.Builder .CanRemoveSkipNavigation(inverse, configurationSource)) { + inverse.SetInverse(null, configurationSource); inverse.DeclaringEntityType.RemoveSkipNavigation(inverse); } @@ -1120,6 +1121,7 @@ public virtual InternalEntityTypeBuilder Ignore([NotNull] string name, Configura && inverse.DeclaringEntityType.Builder .CanRemoveSkipNavigation(inverse, configurationSource)) { + inverse.SetInverse(null, configurationSource); inverse.DeclaringEntityType.RemoveSkipNavigation(inverse); } diff --git a/src/EFCore/Metadata/Internal/InternalModelBuilder.cs b/src/EFCore/Metadata/Internal/InternalModelBuilder.cs index 1a7d8df7877..95fbf275a8c 100644 --- a/src/EFCore/Metadata/Internal/InternalModelBuilder.cs +++ b/src/EFCore/Metadata/Internal/InternalModelBuilder.cs @@ -211,6 +211,46 @@ private InternalEntityTypeBuilder Entity( return entityTypeWithDefiningNavigation?.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. + /// + public virtual InternalModelBuilder RemoveAssociationEntityIfCreatedImplicitly( + [NotNull] EntityType associationEntityType, + bool removeSkipNavigations, + ConfigurationSource configurationSource) + { + Check.NotNull(associationEntityType, nameof(associationEntityType)); + + if (!associationEntityType.IsImplicitlyCreatedAssociationEntityType) + { + return null; + } + + Debug.Assert(associationEntityType.GetForeignKeys().Count() == 2, + "Implicitly created association entity types should have exactly 2 foreign keys"); + foreach (var fk in associationEntityType.GetForeignKeys()) + { + var skipNavigation = fk.GetReferencingSkipNavigations().FirstOrDefault(); + if (skipNavigation != null) + { + skipNavigation.SetForeignKey(null, configurationSource); + skipNavigation.SetInverse(null, configurationSource); + + if (removeSkipNavigations + && fk.PrincipalEntityType.Builder + .CanRemoveSkipNavigation(skipNavigation, configurationSource)) + { + fk.PrincipalEntityType.RemoveSkipNavigation(skipNavigation); + } + } + } + + return HasNoEntityType(associationEntityType, 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 diff --git a/src/EFCore/Metadata/Internal/InternalSkipNavigationBuilder.cs b/src/EFCore/Metadata/Internal/InternalSkipNavigationBuilder.cs index eb82ea328d6..7b8ad1eee58 100644 --- a/src/EFCore/Metadata/Internal/InternalSkipNavigationBuilder.cs +++ b/src/EFCore/Metadata/Internal/InternalSkipNavigationBuilder.cs @@ -70,6 +70,18 @@ public virtual InternalSkipNavigationBuilder HasForeignKey([CanBeNull] ForeignKe foreignKey.UpdateConfigurationSource(configurationSource); } + if (Metadata.AssociationEntityType != null + && foreignKey?.DeclaringEntityType != Metadata.AssociationEntityType) + { + // Have reset the foreign key of a skip navigation on one side of an + // association entity type to a different entity type. An implicit + // association entity type is only useful if both sides are + // configured - so, if it is implicit, remove that entity type + // (which will also remove the other skip navigation's foreign key). + Metadata.AssociationEntityType.Model.Builder.RemoveAssociationEntityIfCreatedImplicitly( + Metadata.AssociationEntityType, removeSkipNavigations: false, configurationSource); + } + Metadata.SetForeignKey(foreignKey, configurationSource); return this; } @@ -98,6 +110,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.IsImplicitlyCreatedAssociationEntityType == 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..961796ed6af 100644 --- a/src/EFCore/Metadata/Internal/Model.cs +++ b/src/EFCore/Metadata/Internal/Model.cs @@ -35,6 +35,11 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Internal /// public class Model : ConventionAnnotatable, IMutableModel, IConventionModel { + /// + /// The CLR type that is used for property bag entity types when no other type is specified. + /// + public static readonly Type DefaultPropertyBagType = typeof(Dictionary); + private readonly SortedDictionary _entityTypes = new SortedDictionary(StringComparer.Ordinal); @@ -754,6 +759,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 +1195,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 b8b505f63a5..c405cbefc02 100644 --- a/src/EFCore/Metadata/Internal/SkipNavigation.cs +++ b/src/EFCore/Metadata/Internal/SkipNavigation.cs @@ -194,9 +194,9 @@ public virtual ForeignKey SetForeignKey([CanBeNull] ForeignKey foreignKey, Confi && Inverse.AssociationEntityType != AssociationEntityType) { throw new InvalidOperationException(CoreStrings.SkipInverseMismatchedForeignKey( - foreignKey.Properties.Format(), - Name, AssociationEntityType.DisplayName(), - Inverse.Name, Inverse.AssociationEntityType.DisplayName())); + 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 a46d53dae8b..fe7db325d17 100644 --- a/src/EFCore/Properties/CoreStrings.Designer.cs +++ b/src/EFCore/Properties/CoreStrings.Designer.cs @@ -2716,6 +2716,14 @@ public static string PrincipalKeylessType([CanBeNull] object entityType, [CanBeN GetString("PrincipalKeylessType", nameof(entityType), nameof(firstNavigationSpecification), nameof(secondNavigationSpecification)), entityType, firstNavigationSpecification, secondNavigationSpecification); + /// + /// 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 3f79f953f4a..908146d6d11 100644 --- a/src/EFCore/Properties/CoreStrings.resx +++ b/src/EFCore/Properties/CoreStrings.resx @@ -1424,4 +1424,7 @@ The entity type '{entityType}' cannot be on the principal end of the relationship between '{firstNavigationSpecification}' and '{secondNavigationSpecification}'. The principal entity type must have a key. + + 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.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs b/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs index f65e38ae1d2..3437b2eaf08 100644 --- a/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs +++ b/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs @@ -260,6 +260,20 @@ private class EntityWithNullableEnumType public Days? Day { get; set; } } + private class ManyToManyLeft + { + public int Id { get; set; } + public string Name { get; set; } + public List Rights { get; set; } + } + + private class ManyToManyRight + { + public int Id { get; set; } + public string Description { get; set; } + public List Lefts { get; set; } + } + private class CustomValueGenerator : ValueGenerator { public override int Next(EntityEntry entry) => throw new NotImplementedException(); @@ -1095,6 +1109,135 @@ public virtual void Foreign_keys_are_stored_in_snapshot() }); } + [ConditionalFact] + public virtual void Many_to_many_join_table_stored_in_snapshot() + { + Test( + builder => + { + builder + .Entity() + .HasMany(l => l.Rights) + .WithMany(r => r.Lefts); + }, + AddBoilerPlate( + GetHeading() + + @" + modelBuilder.Entity(""Join_ManyToManyLeft_ManyToManyRight"", b => + { + b.Property(""ManyToManyLeft_Id"") + .HasColumnType(""int""); + + b.Property(""ManyToManyRight_Id"") + .HasColumnType(""int""); + + b.HasKey(""ManyToManyLeft_Id"", ""ManyToManyRight_Id""); + + b.HasIndex(""ManyToManyRight_Id""); + + b.ToTable(""Join_ManyToManyLeft_ManyToManyRight""); + }); + + modelBuilder.Entity(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+ManyToManyLeft"", b => + { + b.Property(""Id"") + .ValueGeneratedOnAdd() + .HasColumnType(""int"") + .UseIdentityColumn(); + + b.Property(""Name"") + .HasColumnType(""nvarchar(max)""); + + b.HasKey(""Id""); + + b.ToTable(""ManyToManyLeft""); + }); + + modelBuilder.Entity(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+ManyToManyRight"", b => + { + b.Property(""Id"") + .ValueGeneratedOnAdd() + .HasColumnType(""int"") + .UseIdentityColumn(); + + b.Property(""Description"") + .HasColumnType(""nvarchar(max)""); + + b.HasKey(""Id""); + + b.ToTable(""ManyToManyRight""); + }); + + modelBuilder.Entity(""Join_ManyToManyLeft_ManyToManyRight"", b => + { + b.HasOne(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+ManyToManyLeft"", null) + .WithMany() + .HasForeignKey(""ManyToManyLeft_Id"") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + + b.HasOne(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+ManyToManyRight"", null) + .WithMany() + .HasForeignKey(""ManyToManyRight_Id"") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + });", usingSystem: true), + model => + { + var associationEntity = model.FindEntityType("Join_ManyToManyLeft_ManyToManyRight"); + Assert.NotNull(associationEntity); + Assert.Collection(associationEntity.GetDeclaredProperties(), + p => + { + Assert.Equal("ManyToManyLeft_Id", p.Name); + Assert.True(p.IsShadowProperty()); + }, + p => + { + Assert.Equal("ManyToManyRight_Id", p.Name); + Assert.True(p.IsShadowProperty()); + }); + Assert.Collection(associationEntity.FindDeclaredPrimaryKey().Properties, + p => + { + Assert.Equal("ManyToManyLeft_Id", p.Name); + }, + p => + { + Assert.Equal("ManyToManyRight_Id", p.Name); + }); + Assert.Collection(associationEntity.GetDeclaredForeignKeys(), + fk => + { + Assert.Equal("Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+ManyToManyLeft", fk.PrincipalEntityType.Name); + Assert.Collection(fk.PrincipalKey.Properties, + p => + { + Assert.Equal("Id", p.Name); + }); + Assert.Collection(fk.Properties, + p => + { + Assert.Equal("ManyToManyLeft_Id", p.Name); + }); + }, + fk => + { + Assert.Equal("Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+ManyToManyRight", fk.PrincipalEntityType.Name); + Assert.Collection(fk.PrincipalKey.Properties, + p => + { + Assert.Equal("Id", p.Name); + }); + Assert.Collection(fk.Properties, + p => + { + Assert.Equal("ManyToManyRight_Id", p.Name); + }); + }); + }); + } + [ConditionalFact] public virtual void TableName_preserved_when_generic() { 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 et.IsImplicitlyCreatedAssociationEntityType)); + } + + [ConditionalFact] + public void Association_entity_type_is_not_created_when_no_inverse_skip_navigation() + { + var modelBuilder = CreateInternalModeBuilder(); + var manyToManyFirst = modelBuilder.Entity(typeof(ManyToManyFirst), ConfigurationSource.Convention); + var manyToManySecond = modelBuilder.Entity(typeof(ManyToManySecond), ConfigurationSource.Convention); + var manyToManyJoin = modelBuilder.Entity(typeof(ManyToManyJoin), ConfigurationSource.Convention); + + var manyToManyFirstPK = manyToManyFirst.PrimaryKey(new[] { nameof(ManyToManyFirst.Id) }, ConfigurationSource.Convention); + var manyToManySecondPK = 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); + // do not set SkipNav's as inverses of one another + + RunConvention(skipNavOnFirst); + + Assert.Empty(manyToManyFirst.Metadata.Model.GetEntityTypes() + .Where(et => et.IsImplicitlyCreatedAssociationEntityType)); + } + + [ConditionalFact] + public void Association_entity_type_is_not_created_when_skip_navigation_is_not_collection() + { + var modelBuilder = CreateInternalModeBuilder(); + var manyToManyFirst = modelBuilder.Entity(typeof(ManyToManyFirst), ConfigurationSource.Convention); + var manyToManySecond = modelBuilder.Entity(typeof(ManyToManySecond), ConfigurationSource.Convention); + var manyToManyJoin = modelBuilder.Entity(typeof(ManyToManyJoin), ConfigurationSource.Convention); + + var manyToManyFirstPK = manyToManyFirst.PrimaryKey(new[] { nameof(ManyToManyFirst.Id) }, ConfigurationSource.Convention); + var manyToManySecondPK = manyToManySecond.PrimaryKey(new[] { nameof(ManyToManySecond.Id) }, ConfigurationSource.Convention); + + var skipNavOnFirst = manyToManyFirst.HasSkipNavigation( + new MemberIdentity(typeof(ManyToManyFirst).GetProperty(nameof(ManyToManyFirst.Second))), + manyToManySecond.Metadata, + ConfigurationSource.Convention, + collection: false); + var skipNavOnSecond = manyToManySecond.HasSkipNavigation( + new MemberIdentity(typeof(ManyToManySecond).GetProperty(nameof(ManyToManySecond.ManyToManyFirsts))), + manyToManyFirst.Metadata, + ConfigurationSource.Convention); + skipNavOnFirst.HasInverse(skipNavOnSecond.Metadata, ConfigurationSource.Convention); + + RunConvention(skipNavOnFirst); + + Assert.Empty(manyToManyFirst.Metadata.Model.GetEntityTypes() + .Where(et => et.IsImplicitlyCreatedAssociationEntityType)); + } + + [ConditionalFact] + public void Association_entity_type_is_not_created_when_inverse_skip_navigation_is_not_collection() + { + var modelBuilder = CreateInternalModeBuilder(); + var manyToManyFirst = modelBuilder.Entity(typeof(ManyToManyFirst), ConfigurationSource.Convention); + var manyToManySecond = modelBuilder.Entity(typeof(ManyToManySecond), ConfigurationSource.Convention); + var manyToManyJoin = modelBuilder.Entity(typeof(ManyToManyJoin), ConfigurationSource.Convention); + + var manyToManyFirstPK = manyToManyFirst.PrimaryKey(new[] { nameof(ManyToManyFirst.Id) }, ConfigurationSource.Convention); + var manyToManySecondPK = 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.First))), + manyToManyFirst.Metadata, + ConfigurationSource.Convention, + collection: false); + skipNavOnFirst.HasInverse(skipNavOnSecond.Metadata, ConfigurationSource.Convention); + + RunConvention(skipNavOnFirst); + + Assert.Empty(manyToManyFirst.Metadata.Model.GetEntityTypes() + .Where(et => et.IsImplicitlyCreatedAssociationEntityType)); + } + + [ConditionalFact] + public void Association_entity_type_is_not_created_when_skip_navigation_already_in_use() + { + var modelBuilder = CreateInternalModeBuilder(); + var manyToManyFirst = modelBuilder.Entity(typeof(ManyToManyFirst), ConfigurationSource.Convention); + var manyToManySecond = modelBuilder.Entity(typeof(ManyToManySecond), ConfigurationSource.Convention); + var manyToManyJoin = modelBuilder.Entity(typeof(ManyToManyJoin), ConfigurationSource.Convention); + + var manyToManyFirstPK = manyToManyFirst.PrimaryKey(new[] { nameof(ManyToManyFirst.Id) }, ConfigurationSource.Convention); + var manyToManySecondPK = 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); + skipNavOnFirst.HasInverse(skipNavOnSecond.Metadata, ConfigurationSource.Convention); + + // assign a non-null foreign key to skipNavOnFirst to make it appear to be "in use" + var leftFK = manyToManyJoin.HasRelationship( + manyToManyFirst.Metadata.Name, + new[] { nameof(ManyToManyJoin.LeftId) }, + manyToManyFirstPK.Metadata, + ConfigurationSource.Convention); + skipNavOnFirst.Metadata.SetForeignKey(leftFK.Metadata, ConfigurationSource.Convention); + + RunConvention(skipNavOnFirst); + + Assert.Empty(manyToManyFirst.Metadata.Model.GetEntityTypes() + .Where(et => et.IsImplicitlyCreatedAssociationEntityType)); + } + + [ConditionalFact] + public void Association_entity_type_is_not_created_when_inverse_skip_navigation_already_in_use() + { + var modelBuilder = CreateInternalModeBuilder(); + var manyToManyFirst = modelBuilder.Entity(typeof(ManyToManyFirst), ConfigurationSource.Convention); + var manyToManySecond = modelBuilder.Entity(typeof(ManyToManySecond), ConfigurationSource.Convention); + var manyToManyJoin = modelBuilder.Entity(typeof(ManyToManyJoin), ConfigurationSource.Convention); + + var manyToManyFirstPK = manyToManyFirst.PrimaryKey(new[] { nameof(ManyToManyFirst.Id) }, ConfigurationSource.Convention); + var manyToManySecondPK = 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); + skipNavOnFirst.HasInverse(skipNavOnSecond.Metadata, ConfigurationSource.Convention); + + // assign a non-null foreign key to skipNavOnSecond to make it appear to be "in use" + var rightFK = manyToManyJoin.HasRelationship( + manyToManySecond.Metadata.Name, + new[] { nameof(ManyToManyJoin.RightId) }, + manyToManySecondPK.Metadata, + ConfigurationSource.Convention); + skipNavOnSecond.Metadata.SetForeignKey(rightFK.Metadata, ConfigurationSource.Convention); + + RunConvention(skipNavOnFirst); + + Assert.Empty(manyToManyFirst.Metadata.Model.GetEntityTypes() + .Where(et => et.IsImplicitlyCreatedAssociationEntityType)); + } + + [ConditionalFact] + public void Association_entity_type_is_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); + skipNavOnFirst.HasInverse(skipNavOnSecond.Metadata, ConfigurationSource.Convention); + + RunConvention(skipNavOnFirst); + + var joinEntityType = manyToManyFirst.Metadata.Model.GetEntityTypes() + .Single(et => et.IsImplicitlyCreatedAssociationEntityType); + + Assert.Equal("Join_ManyToManyFirst_ManyToManySecond", joinEntityType.Name); + + var skipNavOnManyToManyFirst = manyToManyFirst.Metadata.GetSkipNavigations().Single(); + var skipNavOnManyToManySecond = manyToManySecond.Metadata.GetSkipNavigations().Single(); + Assert.Equal("ManyToManySeconds", skipNavOnManyToManyFirst.Name); + Assert.Equal("ManyToManyFirsts", skipNavOnManyToManySecond.Name); + Assert.Same(skipNavOnManyToManyFirst.Inverse, skipNavOnManyToManySecond); + Assert.Same(skipNavOnManyToManySecond.Inverse, skipNavOnManyToManyFirst); + + var manyToManyFirstForeignKey = skipNavOnManyToManyFirst.ForeignKey; + var manyToManySecondForeignKey = skipNavOnManyToManySecond.ForeignKey; + Assert.NotNull(manyToManyFirstForeignKey); + Assert.NotNull(manyToManySecondForeignKey); + Assert.Equal(2, joinEntityType.GetForeignKeys().Count()); + Assert.Equal(manyToManyFirstForeignKey.DeclaringEntityType, joinEntityType); + Assert.Equal(manyToManySecondForeignKey.DeclaringEntityType, joinEntityType); + + var key = joinEntityType.FindPrimaryKey(); + Assert.Equal( + new[] { + nameof(ManyToManyFirst) + "_" + nameof(ManyToManyFirst.Id), + nameof(ManyToManySecond) + "_" + nameof(ManyToManySecond.Id) }, + key.Properties.Select(p => p.Name)); + } + + public ListLoggerFactory ListLoggerFactory { get; } + = new ListLoggerFactory(l => l == DbLoggerCategory.Model.Name); + + private DiagnosticsLogger CreateLogger() + { + var options = new LoggingOptions(); + options.Initialize(new DbContextOptionsBuilder().EnableSensitiveDataLogging(false).Options); + var modelLogger = new DiagnosticsLogger( + ListLoggerFactory, + options, + new DiagnosticListener("Fake"), + new TestLoggingDefinitions(), + new NullDbContextLogger()); + return modelLogger; + } + + private InternalSkipNavigationBuilder RunConvention(InternalSkipNavigationBuilder skipNavBuilder) + { + var context = new ConventionContext(skipNavBuilder.Metadata.DeclaringEntityType.Model.ConventionDispatcher); + CreateManyToManyConvention().ProcessSkipNavigationAdded(skipNavBuilder, context); + return context.ShouldStopProcessing() ? (InternalSkipNavigationBuilder)context.Result : skipNavBuilder; + } + + private ManyToManyAssociationEntityTypeConvention CreateManyToManyConvention() + => new ManyToManyAssociationEntityTypeConvention(CreateDependencies()); + + private ProviderConventionSetBuilderDependencies CreateDependencies() + => InMemoryTestHelpers.Instance.CreateContextServices().GetRequiredService() + .With(CreateLogger()); + + private InternalModelBuilder CreateInternalModeBuilder() + { + return new InternalModelBuilder(new Model(new ConventionSet())); + } + + private class ManyToManyFirst + { + public int Id { get; set; } + public IEnumerable ManyToManySeconds { get; set; } + public ManyToManySecond Second { get; set; } + } + + private class ManyToManySecond + { + public int Id { get; set; } + public IEnumerable ManyToManyFirsts { get; set; } + public ManyToManyFirst First { get; set; } + } + + private class ManyToManySelf + { + public int Id { get; set; } + public IEnumerable ManyToManySelf1 { get; set; } + public IEnumerable ManyToManySelf2 { get; set; } + } + + private class ManyToManyJoin + { + public int LeftId { get; set; } + public int RightId { get; set; } + } + } +} diff --git a/test/EFCore.Tests/Metadata/Conventions/RelationshipDiscoveryConventionTest.cs b/test/EFCore.Tests/Metadata/Conventions/RelationshipDiscoveryConventionTest.cs index 16acc0b0f80..16072bbe022 100644 --- a/test/EFCore.Tests/Metadata/Conventions/RelationshipDiscoveryConventionTest.cs +++ b/test/EFCore.Tests/Metadata/Conventions/RelationshipDiscoveryConventionTest.cs @@ -239,16 +239,55 @@ public void Many_to_one_bidirectional_is_discovered() } [ConditionalFact] - public void Many_to_many_bidirectional_is_not_discovered() + public void Many_to_many_skip_navigations_are_not_discovered_if_self_association() { - var entityBuilder = CreateInternalEntityBuilder(); + var modelBuilder = CreateInternalModeBuilder(); + var manyToManySelf = modelBuilder.Entity(typeof(ManyToManySelf), ConfigurationSource.Convention); - Assert.Same(entityBuilder, RunConvention(entityBuilder)); - Cleanup(entityBuilder.ModelBuilder); + manyToManySelf.PrimaryKey(new[] { nameof(ManyToManySelf.Id) }, ConfigurationSource.Convention); - Assert.Empty(entityBuilder.Metadata.GetForeignKeys()); - Assert.Empty(entityBuilder.Metadata.GetNavigations()); - Assert.Single(entityBuilder.Metadata.Model.GetEntityTypes()); + RunConvention(manyToManySelf); + + Assert.Empty(manyToManySelf.Metadata.GetSkipNavigations()); + } + + [ConditionalFact] + public void Many_to_many_skip_navigations_are_not_discovered_if_relationship_should_be_on_ancestors() + { + var modelBuilder = CreateInternalModeBuilder(); + var derivedManyToManyFirst = modelBuilder.Entity(typeof(DerivedManyToManyFirst), ConfigurationSource.Convention); + var derivedManyToManySecond = modelBuilder.Entity(typeof(DerivedManyToManySecond), ConfigurationSource.Convention); + + derivedManyToManyFirst.PrimaryKey(new[] { nameof(DerivedManyToManyFirst.Id) }, ConfigurationSource.Convention); + derivedManyToManySecond.PrimaryKey(new[] { nameof(DerivedManyToManySecond.Id) }, ConfigurationSource.Convention); + + RunConvention(derivedManyToManyFirst); + + Assert.Empty(derivedManyToManyFirst.Metadata.GetSkipNavigations()); + Assert.Empty(derivedManyToManySecond.Metadata.GetSkipNavigations()); + } + + [ConditionalFact] + public void Many_to_many_bidirectional_sets_up_skip_navigations_but_not_association_entity_type() + { + 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); + + RunConvention(manyToManyFirst); + + var navigationOnManyToManyFirst = manyToManyFirst.Metadata.GetSkipNavigations().Single(); + var navigationOnManyToManySecond = manyToManySecond.Metadata.GetSkipNavigations().Single(); + Assert.Equal("ManyToManySeconds", navigationOnManyToManyFirst.Name); + Assert.Equal("ManyToManyFirsts", navigationOnManyToManySecond.Name); + Assert.Same(navigationOnManyToManyFirst.Inverse, navigationOnManyToManySecond); + Assert.Same(navigationOnManyToManySecond.Inverse, navigationOnManyToManyFirst); + + Assert.Empty(manyToManyFirst.Metadata.Model.GetEntityTypes() + .Where(et => et.IsImplicitlyCreatedAssociationEntityType)); } [ConditionalFact] @@ -1128,7 +1167,7 @@ private ProviderConventionSetBuilderDependencies CreateDependencies() => InMemoryTestHelpers.Instance.CreateContextServices().GetRequiredService() .With(CreateLogger()); - private InternalEntityTypeBuilder CreateInternalEntityBuilder(params Action[] onEntityAdded) + private InternalModelBuilder CreateInternalModeBuilder(params Action[] onEntityAdded) { var conventions = new ConventionSet(); if (onEntityAdded != null) @@ -1142,7 +1181,12 @@ private InternalEntityTypeBuilder CreateInternalEntityBuilder(params Action(params Action[] onEntityAdded) + { + var modelBuilder = CreateInternalModeBuilder(onEntityAdded); var entityBuilder = modelBuilder.Entity(typeof(T), ConfigurationSource.DataAnnotation); return entityBuilder; @@ -1239,22 +1283,33 @@ public static void IgnoreNavigation(IConventionEntityTypeBuilder entityTypeBuild private class ManyToManyFirst { - public static readonly PropertyInfo NavigationProperty = - typeof(OneToManyPrincipal).GetProperty("ManyToManySeconds", BindingFlags.Public | BindingFlags.Instance); - public int Id { get; set; } public IEnumerable ManyToManySeconds { get; set; } } private class ManyToManySecond { - public static readonly PropertyInfo NavigationProperty = - typeof(OneToManyPrincipal).GetProperty("ManyToManyFirsts", BindingFlags.Public | BindingFlags.Instance); - public int Id { get; set; } public IEnumerable ManyToManyFirsts { get; set; } } + private class ManyToManySelf + { + public int Id { get; set; } + public IEnumerable ManyToManySelf1 { get; set; } + public IEnumerable ManyToManySelf2 { get; set; } + } + + private class DerivedManyToManyFirst : ManyToManyFirst + { + public string Name { get; set; } + } + + private class DerivedManyToManySecond : ManyToManySecond + { + public string Name { get; set; } + } + private class MultipleNavigationsFirst { public static readonly PropertyInfo CollectionNavigationProperty = diff --git a/test/EFCore.Tests/Metadata/Internal/InternalModelBuilderTest.cs b/test/EFCore.Tests/Metadata/Internal/InternalModelBuilderTest.cs index 360f7f97dc4..1faa3027bdd 100644 --- a/test/EFCore.Tests/Metadata/Internal/InternalModelBuilderTest.cs +++ b/test/EFCore.Tests/Metadata/Internal/InternalModelBuilderTest.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; using System.Linq; using System.Reflection; using Microsoft.EntityFrameworkCore.Diagnostics; @@ -347,6 +348,135 @@ public void Can_mark_type_as_owned_type() Assert.Throws(() => modelBuilder.Owned(typeof(Details), ConfigurationSource.Explicit)).Message); } + [ConditionalTheory] + [InlineData(true)] + [InlineData(false)] + public void Can_remove_implicitly_created_association_entity_type(bool removeSkipNavs) + { + 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 manyToManyLeftPK = manyToManyLeft.PrimaryKey(new[] { nameof(ManyToManyLeft.Id) }, ConfigurationSource.Convention); + var manyToManyRightPK = manyToManyRight.PrimaryKey(new[] { nameof(ManyToManyRight.Id) }, 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); + skipNavOnLeft.HasInverse(skipNavOnRight.Metadata, ConfigurationSource.Convention); + + var associationEntityTypeBuilder = + model.AddEntityType( + "AssociationEntity", + typeof(Dictionary), + ConfigurationSource.Convention).Builder; + var leftFK = associationEntityTypeBuilder + .HasRelationship( + manyToManyLeft.Metadata.Name, + new List() { "ManyToManyLeft_Id" }, + manyToManyLeftPK.Metadata, + ConfigurationSource.Convention) + .IsUnique(false, ConfigurationSource.Convention) + .Metadata; + var rightFK = associationEntityTypeBuilder + .HasRelationship( + manyToManyRight.Metadata.Name, + new List() { "ManyToManyRight_Id" }, + manyToManyRightPK.Metadata, + ConfigurationSource.Convention) + .IsUnique(false, ConfigurationSource.Convention) + .Metadata; + skipNavOnLeft.HasForeignKey(leftFK, ConfigurationSource.Convention); + skipNavOnRight.HasForeignKey(rightFK, ConfigurationSource.Convention); + associationEntityTypeBuilder.PrimaryKey( + leftFK.Properties.Concat(rightFK.Properties).ToList(), + ConfigurationSource.Convention); + + var associationEntityType = associationEntityTypeBuilder.Metadata; + + Assert.NotNull(associationEntityType); + + Assert.NotNull(modelBuilder.RemoveAssociationEntityIfCreatedImplicitly( + associationEntityType, removeSkipNavs, ConfigurationSource.Convention)); + + Assert.Empty(model.GetEntityTypes() + .Where(e => e.IsImplicitlyCreatedAssociationEntityType)); + + var leftSkipNav = manyToManyLeft.Metadata.FindDeclaredSkipNavigation(nameof(ManyToManyLeft.Rights)); + var rightSkipNav = manyToManyRight.Metadata.FindDeclaredSkipNavigation(nameof(ManyToManyRight.Lefts)); + if (removeSkipNavs) + { + Assert.Null(leftSkipNav); + Assert.Null(rightSkipNav); + } + else + { + Assert.NotNull(leftSkipNav); + Assert.NotNull(rightSkipNav); + } + } + + [ConditionalTheory] + [InlineData(true)] + [InlineData(false)] + public void Cannot_remove_manually_created_association_entity_type(bool removeSkipNavs) + { + 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 manyToManyJoin = modelBuilder.Entity(typeof(ManyToManyJoin), ConfigurationSource.Convention); + var manyToManyLeftPK = manyToManyLeft.PrimaryKey(new[] { nameof(ManyToManyLeft.Id) }, ConfigurationSource.Convention); + var manyToManyRightPK = manyToManyRight.PrimaryKey(new[] { nameof(ManyToManyRight.Id) }, ConfigurationSource.Convention); + manyToManyJoin.PrimaryKey(new[] { nameof(ManyToManyJoin.LeftId), nameof(ManyToManyJoin.RightId) }, 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); + skipNavOnLeft.HasInverse(skipNavOnRight.Metadata, ConfigurationSource.Convention); + + var leftFK = manyToManyJoin.HasRelationship( + manyToManyLeft.Metadata.Name, + new[] { nameof(ManyToManyJoin.LeftId) }, + manyToManyLeftPK.Metadata, + ConfigurationSource.Convention); + skipNavOnLeft.Metadata.SetForeignKey(leftFK.Metadata, ConfigurationSource.Convention); + var rightFK = manyToManyJoin.HasRelationship( + manyToManyRight.Metadata.Name, + new[] { nameof(ManyToManyJoin.RightId) }, + manyToManyRightPK.Metadata, + ConfigurationSource.Convention); + skipNavOnRight.Metadata.SetForeignKey(rightFK.Metadata, ConfigurationSource.Convention); + skipNavOnLeft.HasInverse(skipNavOnRight.Metadata, ConfigurationSource.Convention); + + var associationEntityType = skipNavOnLeft.Metadata.AssociationEntityType; + Assert.NotNull(associationEntityType); + Assert.Same(associationEntityType, skipNavOnRight.Metadata.AssociationEntityType); + + Assert.Null(modelBuilder.RemoveAssociationEntityIfCreatedImplicitly( + associationEntityType, removeSkipNavs, ConfigurationSource.Convention)); + + var leftSkipNav = manyToManyLeft.Metadata.FindDeclaredSkipNavigation(nameof(ManyToManyLeft.Rights)); + var rightSkipNav = manyToManyRight.Metadata.FindDeclaredSkipNavigation(nameof(ManyToManyRight.Lefts)); + Assert.NotNull(leftSkipNav); + Assert.NotNull(rightSkipNav); + + Assert.Same(leftSkipNav.AssociationEntityType, rightSkipNav.AssociationEntityType); + Assert.Same(manyToManyJoin.Metadata, leftSkipNav.AssociationEntityType); + } + private static void Cleanup(InternalModelBuilder modelBuilder) { new ModelCleanupConvention(CreateDependencies()) @@ -404,5 +534,25 @@ private class Details { public string Name { get; set; } } + + private class ManyToManyLeft + { + public int Id { get; set; } + public List Rights { 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 ManyToManyJoin + { + public int LeftId { get; set; } + public int RightId { get; set; } + } } } diff --git a/test/EFCore.Tests/Metadata/Internal/InternalSkipNavigationBuilderTest.cs b/test/EFCore.Tests/Metadata/Internal/InternalSkipNavigationBuilderTest.cs index ff448e08cb6..a1b0c10d7d7 100644 --- a/test/EFCore.Tests/Metadata/Internal/InternalSkipNavigationBuilderTest.cs +++ b/test/EFCore.Tests/Metadata/Internal/InternalSkipNavigationBuilderTest.cs @@ -126,14 +126,23 @@ public void Can_only_override_lower_or_equal_source_ForeignKey() var builder = CreateInternalSkipNavigationBuilder(); IConventionSkipNavigation metadata = builder.Metadata; - var fk = (ForeignKey)metadata.DeclaringEntityType.Model.Builder.Entity(typeof(OrderProduct)) + // the skip navigation is pointing to the automatically-generated association entity type + var originalFK = metadata.ForeignKey; + Assert.NotNull(originalFK); + Assert.Equal(ConfigurationSource.Convention, metadata.GetForeignKeyConfigurationSource()); + + var orderProductEntity = metadata.DeclaringEntityType.Model.Builder.Entity(typeof(OrderProduct)); + var fk = (ForeignKey)orderProductEntity .HasRelationship(metadata.DeclaringEntityType, nameof(OrderProduct.Order)) .IsUnique(false) .Metadata; - Assert.Null(metadata.ForeignKey); - Assert.Null(metadata.GetForeignKeyConfigurationSource()); + // skip navigation is unaffected by the FK created above + Assert.NotSame(fk, metadata.ForeignKey); + Assert.Same(originalFK, metadata.ForeignKey); + Assert.Equal(ConfigurationSource.Convention, metadata.GetForeignKeyConfigurationSource()); + // now explicitly assign the skip navigation's ForeignKey Assert.True(builder.CanSetForeignKey(fk, ConfigurationSource.DataAnnotation)); Assert.NotNull(builder.HasForeignKey(fk, ConfigurationSource.DataAnnotation)); @@ -161,16 +170,19 @@ public void Can_only_override_lower_or_equal_source_Inverse() var builder = CreateInternalSkipNavigationBuilder(); IConventionSkipNavigation metadata = builder.Metadata; + // the skip navigation is pointing to the automatically-generated + // association entity type and so is its inverse var inverse = (SkipNavigation)metadata.TargetEntityType.Builder.HasSkipNavigation( Product.OrdersProperty, metadata.DeclaringEntityType) .Metadata; - Assert.Null(metadata.Inverse); - Assert.Null(metadata.GetInverseConfigurationSource()); - Assert.Null(inverse.Inverse); - Assert.Null(inverse.GetInverseConfigurationSource()); + Assert.NotNull(metadata.Inverse); + Assert.Equal(ConfigurationSource.Convention, metadata.GetInverseConfigurationSource()); + Assert.NotNull(inverse.Inverse); + Assert.Equal(ConfigurationSource.Convention, inverse.GetInverseConfigurationSource()); + // now explicitly assign the skip navigation's Inverse Assert.True(builder.CanSetInverse(inverse, ConfigurationSource.DataAnnotation)); Assert.NotNull(builder.HasInverse(inverse, ConfigurationSource.DataAnnotation)); diff --git a/test/EFCore.Tests/ModelBuilding/ManyToManyTestBase.cs b/test/EFCore.Tests/ModelBuilding/ManyToManyTestBase.cs index d8303fd0814..68eeb507369 100644 --- a/test/EFCore.Tests/ModelBuilding/ManyToManyTestBase.cs +++ b/test/EFCore.Tests/ModelBuilding/ManyToManyTestBase.cs @@ -5,6 +5,7 @@ using System.Linq; using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Metadata; +using Microsoft.EntityFrameworkCore.Metadata.Internal; using Xunit; // ReSharper disable InconsistentNaming @@ -123,7 +124,66 @@ public virtual void Finds_existing_navigations_and_uses_associated_FK_with_field } [ConditionalFact] - public virtual void Configures_association_type() + public virtual void Association_type_is_automatically_configured_by_convention() + { + var modelBuilder = CreateModelBuilder(); + var model = modelBuilder.Model; + + modelBuilder.Entity(); + + var manyToManyA = model.FindEntityType(typeof(AutomaticManyToManyA)); + var manyToManyB = model.FindEntityType(typeof(AutomaticManyToManyB)); + var joinEntityType = model.GetEntityTypes() + .Where(et => ((EntityType)et).IsImplicitlyCreatedAssociationEntityType) + .Single(); + Assert.Equal("Join_AutomaticManyToManyA_AutomaticManyToManyB", joinEntityType.Name); + + var navigationOnManyToManyA = manyToManyA.GetSkipNavigations().Single(); + var navigationOnManyToManyB = manyToManyB.GetSkipNavigations().Single(); + Assert.Equal("Bs", navigationOnManyToManyA.Name); + Assert.Equal("As", navigationOnManyToManyB.Name); + Assert.Same(navigationOnManyToManyA.Inverse, navigationOnManyToManyB); + Assert.Same(navigationOnManyToManyB.Inverse, navigationOnManyToManyA); + + var manyToManyAForeignKey = navigationOnManyToManyA.ForeignKey; + var manyToManyBForeignKey = navigationOnManyToManyB.ForeignKey; + Assert.NotNull(manyToManyAForeignKey); + Assert.NotNull(manyToManyBForeignKey); + Assert.Equal(2, joinEntityType.GetForeignKeys().Count()); + Assert.Equal(manyToManyAForeignKey.DeclaringEntityType, joinEntityType); + Assert.Equal(manyToManyBForeignKey.DeclaringEntityType, joinEntityType); + + var key = joinEntityType.FindPrimaryKey(); + Assert.Equal( + new[] { + nameof(AutomaticManyToManyA) + "_" + nameof(AutomaticManyToManyA.Id), + nameof(AutomaticManyToManyB) + "_" + nameof(AutomaticManyToManyB.Id) }, + key.Properties.Select(p => p.Name)); + + modelBuilder.FinalizeModel(); + } + + [ConditionalFact] + public virtual void Association_type_is_not_automatically_configured_when_navigations_are_ambiguous() + { + var modelBuilder = CreateModelBuilder(); + var model = modelBuilder.Model; + + modelBuilder.Entity(); + + var hob = model.FindEntityType(typeof(Hob)); + var nob = model.FindEntityType(typeof(Nob)); + Assert.NotNull(hob); + Assert.NotNull(nob); + Assert.Empty(model.GetEntityTypes() + .Where(et => ((EntityType)et).IsImplicitlyCreatedAssociationEntityType)); + + Assert.Empty(hob.GetSkipNavigations()); + Assert.Empty(nob.GetSkipNavigations()); + } + + [ConditionalFact] + public virtual void Can_configure_association_type_using_fluent_api() { var modelBuilder = CreateModelBuilder(); var model = modelBuilder.Model; @@ -189,6 +249,9 @@ public virtual void Throws_for_conflicting_many_to_one_on_left() var modelBuilder = CreateModelBuilder(); var model = modelBuilder.Model; + // make sure we do not set up the automatic many-to-many relationship + modelBuilder.Entity().Ignore(e => e.Products); + modelBuilder.Entity() .HasMany(o => o.Products).WithOne(); @@ -209,6 +272,9 @@ public virtual void Throws_for_conflicting_many_to_one_on_right() var modelBuilder = CreateModelBuilder(); var model = modelBuilder.Model; + // make sure we do not set up the automatic many-to-many relationship + modelBuilder.Entity().Ignore(e => e.Products); + modelBuilder.Entity() .HasMany(o => o.Products).WithOne(); diff --git a/test/EFCore.Tests/ModelBuilding/ManyToOneTestBase.cs b/test/EFCore.Tests/ModelBuilding/ManyToOneTestBase.cs index 73fdad8503c..93d649a35bf 100644 --- a/test/EFCore.Tests/ModelBuilding/ManyToOneTestBase.cs +++ b/test/EFCore.Tests/ModelBuilding/ManyToOneTestBase.cs @@ -1611,6 +1611,9 @@ public virtual void Removes_existing_unidirectional_one_to_one_relationship() .HasForeignKey( e => new { e.HobId1, e.HobId2 }); + // The below means the relationship is no longer + // using Nob.Hob. After that it is allowed to override + // Hob.Nob's inverse in the HasOne().WithMany() call below. modelBuilder.Entity().HasOne().WithOne(e => e.Nob); var dependentType = model.FindEntityType(typeof(Hob)); @@ -1622,6 +1625,7 @@ public virtual void Removes_existing_unidirectional_one_to_one_relationship() modelBuilder.FinalizeModel(); + // assert 1:N relationship defined through the HasOne().WithMany() call above var fk = dependentType.GetForeignKeys().Single(); Assert.False(fk.IsUnique); Assert.Same(fk, dependentType.GetNavigations().Single(n => n.Name == nameof(Hob.Nob)).ForeignKey); @@ -1629,6 +1633,9 @@ public virtual void Removes_existing_unidirectional_one_to_one_relationship() Assert.Same(principalKey, principalType.FindPrimaryKey()); Assert.Same(dependentKey, dependentType.FindPrimaryKey()); + // The 1:N relationship above has "used up" Hob.Nob and Nob.Hobs, + // so now the RelationshipDiscoveryConvention should be able + // to unambiguously and automatically match up Nob.Hob and Hob.Nobs var oldFk = principalType.GetForeignKeys().Single(); AssertEqual(new[] { nameof(Nob.HobId1), nameof(Nob.HobId2) }, oldFk.Properties.Select(p => p.Name)); Assert.Same(oldFk, dependentType.GetNavigations().Single(n => n.Name == nameof(Hob.Nobs)).ForeignKey); diff --git a/test/EFCore.Tests/ModelBuilding/OneToManyTestBase.cs b/test/EFCore.Tests/ModelBuilding/OneToManyTestBase.cs index a391c44ad1d..e9303de9584 100644 --- a/test/EFCore.Tests/ModelBuilding/OneToManyTestBase.cs +++ b/test/EFCore.Tests/ModelBuilding/OneToManyTestBase.cs @@ -1844,7 +1844,7 @@ public virtual void Throws_on_existing_many_to_many() nameof(Category) + "." + nameof(Category.Products), nameof(Product), nameof(Category) + "." + nameof(Category.Products), - nameof(Product)), + nameof(Product) + "." + nameof(Product.Categories)), Assert.Throws( () => modelBuilder.Entity() .HasMany(o => o.Products).WithOne()).Message); @@ -1855,8 +1855,17 @@ public virtual void Throws_on_existing_one_to_one_relationship() { var modelBuilder = HobNobBuilder(); var model = modelBuilder.Model; + + // set up a 1:1 relationship using Nob.Hob and Hob.Nob modelBuilder.Entity().HasOne(e => e.Hob).WithOne(e => e.Nob); + // Now that Nob.Hob and Hob.Nob are used, Nob.Hobs and Hob.Nobs + // are no longer ambiguous and we do implicitly create the N:N + // relationship between them. But this can be silently overridden + // by the more explicit HasMany().WithOne() call below. So we do + // not see a clash with that relationship, only with the 1:1 + // relationship configured above. + var dependentType = model.FindEntityType(typeof(Hob)); var principalType = model.FindEntityType(typeof(Nob)); @@ -1878,6 +1887,9 @@ public virtual void Removes_existing_unidirectional_one_to_one_relationship() var model = modelBuilder.Model; modelBuilder.Entity().HasOne(e => e.Hob).WithOne(e => e.Nob); + // The below ensures that the relationship is no longer + // using Nob.Hob. After that it is allowed to override + // Hob.Nob's inverse in the HasMany().WithOne() call below. modelBuilder.Entity().HasOne().WithOne(e => e.Nob); var dependentType = model.FindEntityType(typeof(Hob)); @@ -1887,10 +1899,16 @@ public virtual void Removes_existing_unidirectional_one_to_one_relationship() modelBuilder.Entity().HasMany(e => e.Hobs).WithOne(e => e.Nob); + // assert the 1:N relationship defined through the HasMany().WithOne() call above var fk = dependentType.GetForeignKeys().Single(); Assert.False(fk.IsUnique); Assert.Equal(nameof(Nob.Hobs), fk.PrincipalToDependent.Name); Assert.Equal(nameof(Hob.Nob), fk.DependentToPrincipal.Name); + + // The 1:N relationship above has "used up" Hob.Nob and Nob.Hobs, + // so now the RelationshipDiscoveryConvention should be able + // to unambiguously and automatically match up Nob.Hob and Hob.Nobs + // in a different 1:N relationship. var otherFk = principalType.GetForeignKeys().Single(); Assert.False(fk.IsUnique); Assert.Equal(nameof(Hob.Nobs), otherFk.PrincipalToDependent.Name); diff --git a/test/EFCore.Tests/ModelBuilding/TestModel.cs b/test/EFCore.Tests/ModelBuilding/TestModel.cs index 4b8e1f61615..d5b94963b2e 100644 --- a/test/EFCore.Tests/ModelBuilding/TestModel.cs +++ b/test/EFCore.Tests/ModelBuilding/TestModel.cs @@ -1035,5 +1035,22 @@ public class OneToOneOwnedWithField public OneToOneOwnerWithField OneToOneOwner { get; set; } } + + public class AutomaticManyToManyA + { + public int Id { get; set; } + public string Name { get; set; } + + public List Bs { get; set; } + } + + + public class AutomaticManyToManyB + { + public int Id { get; set; } + public string Name { get; set; } + + public List As { get; set; } + } } }