Use default property names for join entity type

Remove implicit join entity type in a convention
Configure PK for join entity type in a convention
Consolidate HasNoSkipNavigation usage

Fixes #21567

src/EFCore/Extensions/ConventionEntityTypeExtensions.cs

@@ -509,7 +509,13 @@ public static IConventionProperty AddProperty(
         public static IConventionProperty AddProperty(
             [NotNull] this IConventionEntityType entityType, [NotNull] string name, [NotNull] Type propertyType,
             bool setTypeConfigurationSource = true, bool fromDataAnnotation = false)
-            => entityType.AddProperty(name, propertyType, null, setTypeConfigurationSource, fromDataAnnotation);
+            => ((EntityType)entityType).AddProperty(
+                name,
+                propertyType,
+                setTypeConfigurationSource
+                    ? fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention
+                    : (ConfigurationSource?)null,
+                fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention);
 
         /// <summary>
         ///     Adds a property backed by and indexer to this entity type.

src/EFCore/Extensions/MutableEntityTypeExtensions.cs

@@ -470,7 +470,7 @@ public static IMutableProperty AddProperty(
         /// <returns> The newly created property. </returns>
         public static IMutableProperty AddProperty(
             [NotNull] this IMutableEntityType entityType, [NotNull] string name, [NotNull] Type propertyType)
-            => entityType.AddProperty(name, propertyType, null);
+            => ((EntityType)entityType).AddProperty(name, propertyType, ConfigurationSource.Explicit, ConfigurationSource.Explicit);
 
         /// <summary>
         ///     Adds a property backed up by an indexer to this entity type.

src/EFCore/Metadata/Builders/CollectionCollectionBuilder.cs

@@ -118,21 +118,20 @@ public virtual EntityTypeBuilder UsingEntity(
             Check.NotNull(configureRight, nameof(configureRight));
             Check.NotNull(configureLeft, nameof(configureLeft));
 
-            var existingjoinEntityType = (EntityType)
+            var existingJoinEntityType = (EntityType)
                 (LeftNavigation.ForeignKey?.DeclaringEntityType
                     ?? RightNavigation.ForeignKey?.DeclaringEntityType);
             EntityType joinEntityType = null;
-            if (existingjoinEntityType != null)
+            if (existingJoinEntityType != null)
             {
-                if (existingjoinEntityType.ClrType == joinEntityClrType
-                    && !existingjoinEntityType.HasSharedClrType)
+                if (existingJoinEntityType.ClrType == joinEntityClrType
+                    && !existingJoinEntityType.HasSharedClrType)
                 {
-                    joinEntityType = existingjoinEntityType;
+                    joinEntityType = existingJoinEntityType;
                 }
                 else
                 {
-                    ModelBuilder.RemoveJoinEntityIfCreatedImplicitly(
-                        existingjoinEntityType, removeSkipNavigations: false, ConfigurationSource.Explicit);
+                    ModelBuilder.RemoveImplicitJoinEntity(existingJoinEntityType);
                 }
             }
 
@@ -183,8 +182,7 @@ public virtual EntityTypeBuilder UsingEntity(
                 }
                 else
                 {
-                    ModelBuilder.RemoveJoinEntityIfCreatedImplicitly(
-                        existingJoinEntityType, removeSkipNavigations: false, ConfigurationSource.Explicit);
+                    ModelBuilder.RemoveImplicitJoinEntity(existingJoinEntityType);
                 }
             }
 
@@ -281,13 +279,13 @@ protected virtual void Using([NotNull] IMutableForeignKey rightForeignKey, [NotN
             var leftBuilder = ((SkipNavigation)LeftNavigation).Builder;
             var rightBuilder = ((SkipNavigation)RightNavigation).Builder;
 
+            leftBuilder = leftBuilder.HasInverse(rightBuilder.Metadata, ConfigurationSource.Explicit);
+
             leftBuilder = leftBuilder.HasForeignKey((ForeignKey)leftForeignKey, ConfigurationSource.Explicit);
             rightBuilder = rightBuilder.HasForeignKey((ForeignKey)rightForeignKey, ConfigurationSource.Explicit);
 
-            leftBuilder = leftBuilder.HasInverse(rightBuilder.Metadata, ConfigurationSource.Explicit);
-
             LeftNavigation = leftBuilder.Metadata;
-            RightNavigation = leftBuilder.Metadata.Inverse;
+            RightNavigation = rightBuilder.Metadata;
         }
 
         #region Hidden System.Object members

src/EFCore/Metadata/Builders/CollectionCollectionBuilder`.cs

@@ -88,8 +88,7 @@ public virtual EntityTypeBuilder<TJoinEntity> UsingEntity<TJoinEntity>(
                 }
                 else
                 {
-                    ModelBuilder.RemoveJoinEntityIfCreatedImplicitly(
-                        existingJoinEntityType, removeSkipNavigations: false, ConfigurationSource.Explicit);
+                    ModelBuilder.RemoveImplicitJoinEntity(existingJoinEntityType);
                 }
             }
 
@@ -139,8 +138,7 @@ public virtual EntityTypeBuilder<TJoinEntity> UsingEntity<TJoinEntity>(
                 }
                 else
                 {
-                    ModelBuilder.RemoveJoinEntityIfCreatedImplicitly(
-                        existingJoinEntityType, removeSkipNavigations: false, ConfigurationSource.Explicit);
+                    ModelBuilder.RemoveImplicitJoinEntity(existingJoinEntityType);
                 }
             }
 

src/EFCore/Metadata/Builders/CollectionNavigationBuilder.cs

@@ -170,11 +170,14 @@ private InternalForeignKeyBuilder WithOneBuilder(MemberIdentity reference)
 
                 var navigationName = SkipNavigation.Name;
                 var declaringEntityType = (EntityType)DeclaringEntityType;
-                declaringEntityType.Model.Builder
-                    .RemoveJoinEntityIfCreatedImplicitly(
-                        (EntityType)SkipNavigation.JoinEntityType,
-                        removeSkipNavigations: true,
-                        ConfigurationSource.Explicit);
+
+                if (SkipNavigation.Inverse != null)
+                {
+                    ((EntityType)SkipNavigation.Inverse.DeclaringEntityType).Builder.HasNoSkipNavigation(
+                        (SkipNavigation)SkipNavigation.Inverse, ConfigurationSource.Explicit);
+                }
+
+                declaringEntityType.Builder.HasNoSkipNavigation((SkipNavigation)SkipNavigation, ConfigurationSource.Explicit);
 
                 Builder = declaringEntityType.Builder
                     .HasRelationship(

src/EFCore/Metadata/Builders/IConventionEntityTypeBuilder.cs

@@ -584,6 +584,25 @@ IConventionEntityTypeBuilder HasNoRelationship(
         /// <returns> <see langword="true" /> if the foreign key can be removed from this entity type. </returns>
         bool CanRemoveRelationship([NotNull] IConventionForeignKey foreignKey, bool fromDataAnnotation = false);
 
+        /// <summary>
+        ///     Removes a skip navigation from this entity type.
+        /// </summary>
+        /// <param name="skipNavigation"> The skip navigation to be removed. </param>
+        /// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
+        /// <returns>
+        ///     The same builder instance if the skip navigation was removed,
+        ///     <see langword="null" /> otherwise.
+        /// </returns>
+        IConventionEntityTypeBuilder HasNoSkipNavigation([NotNull] ISkipNavigation skipNavigation, bool fromDataAnnotation = false);
+
+        /// <summary>
+        ///     Returns a value indicating whether the skip navigation can be removed from this entity type.
+        /// </summary>
+        /// <param name="skipNavigation"> The skip navigation to be removed. </param>
+        /// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
+        /// <returns> <see langword="true" /> if the skip navigation can be removed from this entity type. </returns>
+        bool CanRemoveSkipNavigation([NotNull] ISkipNavigation skipNavigation, bool fromDataAnnotation = false);
+
         /// <summary>
         ///     Returns a value indicating whether the given navigation can be added to this entity type.
         /// </summary>

src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs

@@ -179,11 +179,18 @@ public virtual ConventionSet CreateConventionSet()
             conventionSet.NavigationAddedConventions.Add(relationshipDiscoveryConvention);
             conventionSet.NavigationAddedConventions.Add(foreignKeyAttributeConvention);
 
+            var manyToManyJoinEntityTypeConvention = new ManyToManyJoinEntityTypeConvention(Dependencies);
             conventionSet.SkipNavigationAddedConventions.Add(backingFieldConvention);
+            conventionSet.SkipNavigationAddedConventions.Add(manyToManyJoinEntityTypeConvention);
 
-            conventionSet.NavigationRemovedConventions.Add(relationshipDiscoveryConvention);
+            conventionSet.SkipNavigationRemovedConventions.Add(manyToManyJoinEntityTypeConvention);
+
+            conventionSet.SkipNavigationInverseChangedConventions.Add(manyToManyJoinEntityTypeConvention);
 
-            conventionSet.SkipNavigationAddedConventions.Add(new ManyToManyJoinEntityTypeConvention(Dependencies));
+            conventionSet.SkipNavigationForeignKeyChangedConventions.Add(manyToManyJoinEntityTypeConvention);
+            conventionSet.SkipNavigationForeignKeyChangedConventions.Add(keyDiscoveryConvention);            
+
+            conventionSet.NavigationRemovedConventions.Add(relationshipDiscoveryConvention);
 
             conventionSet.IndexAddedConventions.Add(foreignKeyIndexConvention);
 

src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.ImmediateConventionScope.cs

@@ -779,13 +779,6 @@ public override IConventionForeignKey OnSkipNavigationForeignKeyChanged(
                     _foreignKeyConventionContext.ResetState(foreignKey);
                     foreach (var skipNavigationConvention in _conventionSet.SkipNavigationForeignKeyChangedConventions)
                     {
-                        if (navigationBuilder.Metadata.Builder == null
-                            || navigationBuilder.Metadata.ForeignKey != foreignKey)
-                        {
-                            Check.DebugAssert(false, "Foreign key changed");
-                            return null;
-                        }
-
                         skipNavigationConvention.ProcessSkipNavigationForeignKeyChanged(
                             navigationBuilder, foreignKey, oldForeignKey, _foreignKeyConventionContext);
                         if (_foreignKeyConventionContext.ShouldStopProcessing())
@@ -824,13 +817,6 @@ public override IConventionSkipNavigation OnSkipNavigationInverseChanged(
                     _skipNavigationConventionContext.ResetState(inverse);
                     foreach (var skipNavigationConvention in _conventionSet.SkipNavigationInverseChangedConventions)
                     {
-                        if (navigationBuilder.Metadata.Builder == null
-                            || navigationBuilder.Metadata.Inverse != inverse)
-                        {
-                            Check.DebugAssert(false, "inverse changed");
-                            return null;
-                        }
-
                         skipNavigationConvention.ProcessSkipNavigationInverseChanged(
                             navigationBuilder, inverse, oldInverse, _skipNavigationConventionContext);
                         if (_skipNavigationConventionContext.ShouldStopProcessing())

src/EFCore/Metadata/Conventions/KeyDiscoveryConvention.cs

@@ -8,7 +8,6 @@
 using Microsoft.EntityFrameworkCore.Diagnostics;
 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
@@ -37,7 +36,8 @@ public class KeyDiscoveryConvention :
         IForeignKeyRemovedConvention,
         IForeignKeyPropertiesChangedConvention,
         IForeignKeyUniquenessChangedConvention,
-        IForeignKeyOwnershipChangedConvention
+        IForeignKeyOwnershipChangedConvention,
+        ISkipNavigationForeignKeyChangedConvention
     {
         private const string KeySuffix = "Id";
 
@@ -120,6 +120,16 @@ protected virtual void TryConfigurePrimaryKey([NotNull] IConventionEntityTypeBui
                 keyProperties.Add(extraProperty);
             }
 
+            if (keyProperties.Count == 0)
+            {
+                var manyToManyForeignKeys = entityType.GetForeignKeys()
+                    .Where(fk => fk.GetReferencingSkipNavigations().Any(n => n.IsCollection)).ToList();
+                if (manyToManyForeignKeys.Count() == 2)
+                {
+                    keyProperties.AddRange(manyToManyForeignKeys.SelectMany(fk => fk.Properties));
+                }
+            }
+
             ProcessKeyProperties(keyProperties, entityType);
 
             if (keyProperties.Count > 0)
@@ -166,23 +176,13 @@ public static IEnumerable<IConventionProperty> DiscoverKeyProperties(
             // ReSharper restore PossibleMultipleEnumeration
         }
 
-        /// <summary>
-        ///     Called after an entity type is added to the model.
-        /// </summary>
-        /// <param name="entityTypeBuilder"> The builder for the entity type. </param>
-        /// <param name="context"> Additional information associated with convention execution. </param>
+        /// <inheritdoc />
         public virtual void ProcessEntityTypeAdded(
             IConventionEntityTypeBuilder entityTypeBuilder,
             IConventionContext<IConventionEntityTypeBuilder> context)
             => TryConfigurePrimaryKey(entityTypeBuilder);
 
-        /// <summary>
-        ///     Called after the base type of an entity type changes.
-        /// </summary>
-        /// <param name="entityTypeBuilder"> The builder for the entity type. </param>
-        /// <param name="newBaseType"> The new base entity type. </param>
-        /// <param name="oldBaseType"> The old base entity type. </param>
-        /// <param name="context"> Additional information associated with convention execution. </param>
+        /// <inheritdoc />
         public virtual void ProcessEntityTypeBaseTypeChanged(
             IConventionEntityTypeBuilder entityTypeBuilder,
             IConventionEntityType newBaseType,
@@ -197,23 +197,14 @@ public virtual void ProcessEntityTypeBaseTypeChanged(
             TryConfigurePrimaryKey(entityTypeBuilder);
         }
 
-        /// <summary>
-        ///     Called after a property is added to the entity type.
-        /// </summary>
-        /// <param name="propertyBuilder"> The builder for the property. </param>
-        /// <param name="context"> Additional information associated with convention execution. </param>
+        /// <inheritdoc />
         public virtual void ProcessPropertyAdded(
             IConventionPropertyBuilder propertyBuilder, IConventionContext<IConventionPropertyBuilder> context)
         {
             TryConfigurePrimaryKey(propertyBuilder.Metadata.DeclaringEntityType.Builder);
         }
 
-        /// <summary>
-        ///     Called after a key is removed.
-        /// </summary>
-        /// <param name="entityTypeBuilder"> The builder for the entity type. </param>
-        /// <param name="key"> The removed key. </param>
-        /// <param name="context"> Additional information associated with convention execution. </param>
+        /// <inheritdoc />
         public virtual void ProcessKeyRemoved(
             IConventionEntityTypeBuilder entityTypeBuilder, IConventionKey key, IConventionContext<IConventionKey> context)
         {
@@ -223,11 +214,7 @@ public virtual void ProcessKeyRemoved(
             }
         }
 
-        /// <summary>
-        ///     Called after a foreign key is added to the entity type.
-        /// </summary>
-        /// <param name="relationshipBuilder"> The builder for the foreign key. </param>
-        /// <param name="context"> Additional information associated with convention execution. </param>
+        /// <inheritdoc />
         public virtual void ProcessForeignKeyAdded(
             IConventionForeignKeyBuilder relationshipBuilder,
             IConventionContext<IConventionForeignKeyBuilder> context)
@@ -238,13 +225,7 @@ public virtual void ProcessForeignKeyAdded(
             }
         }
 
-        /// <summary>
-        ///     Called after the foreign key properties or principal key are changed.
-        /// </summary>
-        /// <param name="relationshipBuilder"> The builder for the foreign key. </param>
-        /// <param name="oldDependentProperties"> The old foreign key properties. </param>
-        /// <param name="oldPrincipalKey"> The old principal key. </param>
-        /// <param name="context"> Additional information associated with convention execution. </param>
+        /// <inheritdoc />
         public virtual void ProcessForeignKeyPropertiesChanged(
             IConventionForeignKeyBuilder relationshipBuilder,
             IReadOnlyList<IConventionProperty> oldDependentProperties,
@@ -260,24 +241,16 @@ public virtual void ProcessForeignKeyPropertiesChanged(
             }
         }
 
-        /// <summary>
-        ///     Called after the ownership value for a foreign key is changed.
-        /// </summary>
-        /// <param name="relationshipBuilder"> The builder for the foreign key. </param>
-        /// <param name="context"> Additional information associated with convention execution. </param>
+
+        /// <inheritdoc />
         public virtual void ProcessForeignKeyOwnershipChanged(
             IConventionForeignKeyBuilder relationshipBuilder,
             IConventionContext<bool?> context)
         {
             TryConfigurePrimaryKey(relationshipBuilder.Metadata.DeclaringEntityType.Builder);
         }
 
-        /// <summary>
-        ///     Called after a foreign key is removed.
-        /// </summary>
-        /// <param name="entityTypeBuilder"> The builder for the entity type. </param>
-        /// <param name="foreignKey"> The removed foreign key. </param>
-        /// <param name="context"> Additional information associated with convention execution. </param>
+        /// <inheritdoc />
         public virtual void ProcessForeignKeyRemoved(
             IConventionEntityTypeBuilder entityTypeBuilder,
             IConventionForeignKey foreignKey,
@@ -289,11 +262,7 @@ public virtual void ProcessForeignKeyRemoved(
             }
         }
 
-        /// <summary>
-        ///     Called after the uniqueness for a foreign key is changed.
-        /// </summary>
-        /// <param name="relationshipBuilder"> The builder for the foreign key. </param>
-        /// <param name="context"> Additional information associated with convention execution. </param>
+        /// <inheritdoc />
         public virtual void ProcessForeignKeyUniquenessChanged(
             IConventionForeignKeyBuilder relationshipBuilder,
             IConventionContext<bool?> context)
@@ -303,5 +272,20 @@ public virtual void ProcessForeignKeyUniquenessChanged(
                 TryConfigurePrimaryKey(relationshipBuilder.Metadata.DeclaringEntityType.Builder);
             }
         }
+
+        /// <inheritdoc />
+        public virtual void ProcessSkipNavigationForeignKeyChanged(
+            IConventionSkipNavigationBuilder skipNavigationBuilder,
+            IConventionForeignKey foreignKey,
+            IConventionForeignKey oldForeignKey,
+            IConventionContext<IConventionForeignKey> context)
+        {
+            var joinEntityTypeBuilder = skipNavigationBuilder.Metadata.ForeignKey?.DeclaringEntityType.Builder;
+            if (joinEntityTypeBuilder != null
+                && skipNavigationBuilder.Metadata.IsCollection)
+            {
+                TryConfigurePrimaryKey(joinEntityTypeBuilder);
+            }
+        }
     }
 }