From ffd97cdd1d7a9d872adca5758bb2eaaef8e57387 Mon Sep 17 00:00:00 2001
From: Andriy Svyryd <Andriy.Svyryd@microsoft.com>
Date: Tue, 19 Dec 2023 13:30:40 -0800
Subject: [PATCH] Break up KeyDiscoveryConvention

Part of #28866
---
 .../Conventions/KeyDiscoveryConvention.cs     | 76 ++++++++++++-------
 1 file changed, 50 insertions(+), 26 deletions(-)

diff --git a/src/EFCore/Metadata/Conventions/KeyDiscoveryConvention.cs b/src/EFCore/Metadata/Conventions/KeyDiscoveryConvention.cs
index 14729e821a8..f1bbd2303e9 100644
--- a/src/EFCore/Metadata/Conventions/KeyDiscoveryConvention.cs
+++ b/src/EFCore/Metadata/Conventions/KeyDiscoveryConvention.cs
@@ -18,6 +18,9 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Conventions;
 ///         is configured using the foreign key properties with an extra property that matches the naming convention above.
 ///     </para>
 ///     <para>
+///         If the entity type is a many-to-many join entity type then the many-to-many foreign key properties are used.
+///     </para>
+///     <para>
 ///         See <see href="https://aka.ms/efcore-docs-conventions">Model building conventions</see> for more information and examples.
 ///     </para>
 /// </remarks>
@@ -57,17 +60,43 @@ public KeyDiscoveryConvention(ProviderConventionSetBuilderDependencies dependenc
     protected virtual void TryConfigurePrimaryKey(IConventionEntityTypeBuilder entityTypeBuilder)
     {
         var entityType = entityTypeBuilder.Metadata;
-        if (entityType.BaseType != null
-            || (entityType.IsKeyless && entityType.GetIsKeylessConfigurationSource() != ConfigurationSource.Convention)
-            || !entityTypeBuilder.CanSetPrimaryKey((IReadOnlyList<IConventionProperty>?)null))
+        if (!ShouldDiscoverKeyProperties(entityType))
         {
             return;
         }
 
+        var keyProperties = DiscoverKeyProperties(entityType);
+        if (keyProperties != null)
+        {
+            ProcessKeyProperties(keyProperties, entityType);
+
+            if (keyProperties.Count > 0)
+            {
+                entityTypeBuilder.PrimaryKey(keyProperties);
+            }
+        }
+    }
+
+    /// <summary>
+    ///     Determines whether key properties should be discovered for the entity type.
+    /// </summary>
+    /// <param name="entityType">The entity type.</param>
+    /// <returns><see langword="true"/> if key properties should be discovered, otherwise <see langword="false"/>.</returns>
+    protected virtual bool ShouldDiscoverKeyProperties(IConventionEntityType entityType) =>
+        entityType.BaseType == null
+            && (!entityType.IsKeyless || entityType.GetIsKeylessConfigurationSource() == ConfigurationSource.Convention)
+            && entityType.Builder.CanSetPrimaryKey((IReadOnlyList<IConventionProperty>?)null);
+
+    /// <summary>
+    ///     Returns the properties that should be used for the primary key.
+    /// </summary>
+    /// <param name="entityType">The entity type.</param>
+    /// <returns>The properties that should be used for the primary key.</returns>
+    protected virtual List<IConventionProperty>? DiscoverKeyProperties(IConventionEntityType entityType)
+    {
         List<IConventionProperty>? keyProperties = null;
         var ownership = entityType.FindOwnership();
-        if (ownership != null
-            && ownership.DeclaringEntityType != entityType)
+        if (ownership?.DeclaringEntityType != entityType)
         {
             ownership = null;
         }
@@ -86,7 +115,7 @@ protected virtual void TryConfigurePrimaryKey(IConventionEntityTypeBuilder entit
             if (keyProperties.Count > 1)
             {
                 Dependencies.Logger.MultiplePrimaryKeyCandidates(keyProperties[0], keyProperties[1]);
-                return;
+                return null;
             }
         }
 
@@ -101,7 +130,7 @@ protected virtual void TryConfigurePrimaryKey(IConventionEntityTypeBuilder entit
                     || primaryKey!.Properties.Count == 1
                     || ownership.Properties.Contains(shadowProperty))
                 {
-                    shadowProperty = entityTypeBuilder.CreateUniqueProperty(typeof(int), "Id", required: true)!.Metadata;
+                    shadowProperty = entityType.Builder.CreateUniqueProperty(typeof(int), "Id", required: true)!.Metadata;
                 }
 
                 keyProperties.Clear();
@@ -125,6 +154,19 @@ protected virtual void TryConfigurePrimaryKey(IConventionEntityTypeBuilder entit
             }
         }
 
+        return keyProperties;
+    }
+
+    /// <summary>
+    ///     Adds or removes properties to be used for the primary key.
+    /// </summary>
+    /// <param name="keyProperties">The properties that will be used to configure the key.</param>
+    /// <param name="entityType">The entity type being configured.</param>
+    protected virtual void ProcessKeyProperties(
+        IList<IConventionProperty> keyProperties,
+        IConventionEntityType entityType)
+    {
+        // Remove duplicates
         for (var i = keyProperties.Count - 1; i >= 0; i--)
         {
             var property = keyProperties[i];
@@ -137,24 +179,6 @@ protected virtual void TryConfigurePrimaryKey(IConventionEntityTypeBuilder entit
                 }
             }
         }
-
-        ProcessKeyProperties(keyProperties, entityType);
-
-        if (keyProperties.Count > 0)
-        {
-            entityTypeBuilder.PrimaryKey(keyProperties);
-        }
-    }
-
-    /// <summary>
-    ///     Adds or removes properties to be used for the primary key.
-    /// </summary>
-    /// <param name="keyProperties">The properties that will be used to configure the key.</param>
-    /// <param name="entityType">The entity type being configured.</param>
-    protected virtual void ProcessKeyProperties(
-        IList<IConventionProperty> keyProperties,
-        IConventionEntityType entityType)
-    {
     }
 
     /// <summary>
@@ -179,9 +203,9 @@ public static IEnumerable<IConventionProperty> DiscoverKeyProperties(
                     && p.Name.StartsWith(entityTypeName, StringComparison.OrdinalIgnoreCase)
                     && p.Name.EndsWith(KeySuffix, StringComparison.OrdinalIgnoreCase));
         }
+        // ReSharper restore PossibleMultipleEnumeration
 
         return keyProperties;
-        // ReSharper restore PossibleMultipleEnumeration
     }
 
     /// <summary>