PR feedback

src/EFCore.Relational/Metadata/Conventions/EntityTypeHierarchyMappingConvention.cs

@@ -45,7 +45,7 @@ public virtual void ProcessModelFinalizing(IConventionModelBuilder modelBuilder,
                 {
                     var pk = entityType.FindPrimaryKey();
                     if (pk != null
-                        && !entityType.FindForeignKeys(pk.Properties)
+                        && !entityType.FindDeclaredForeignKeys(pk.Properties)
                         .Any(fk => fk.PrincipalKey.IsPrimaryKey() && fk.PrincipalEntityType.IsAssignableFrom(entityType)))
                     {
                         entityType.Builder.HasRelationship(entityType.BaseType, pk.Properties, entityType.BaseType.FindPrimaryKey())

src/EFCore.SqlServer/Extensions/SqlServerPropertyExtensions.cs

@@ -336,7 +336,7 @@ public static SqlServerValueGenerationStrategy GetValueGenerationStrategy([NotNu
             }
 
             if (property.ValueGenerated != ValueGenerated.OnAdd
-                || property.GetContainingForeignKeys().Any(IsNonReduntant)
+                || property.GetContainingForeignKeys().Any(fk => !fk.IsBaseLinking())
                 || property.GetDefaultValue(storeObject) != null
                 || property.GetDefaultValueSql(storeObject) != null
                 || property.GetComputedColumnSql(storeObject) != null)
@@ -345,11 +345,6 @@ public static SqlServerValueGenerationStrategy GetValueGenerationStrategy([NotNu
             }
 
             return GetDefaultValueGenerationStrategy(property);
-
-            static bool IsNonReduntant(IForeignKey foreignKey)
-                => !(foreignKey.PrincipalKey.IsPrimaryKey()
-                    && foreignKey.PrincipalEntityType.IsAssignableFrom(foreignKey.DeclaringEntityType)
-                    && foreignKey.Properties.SequenceEqual(foreignKey.PrincipalKey.Properties));
         }
 
         private static SqlServerValueGenerationStrategy GetDefaultValueGenerationStrategy(IProperty property)

src/EFCore.SqlServer/Metadata/Conventions/SqlServerOnDeleteConvention.cs

@@ -46,9 +46,7 @@ protected override DeleteBehavior GetTargetDeleteBehavior(IConventionForeignKey
                 return deleteBehavior;
             }
 
-            if (foreignKey.PrincipalEntityType.IsAssignableFrom(foreignKey.DeclaringEntityType)
-                && foreignKey.PrincipalKey.IsPrimaryKey()
-                && foreignKey.Properties.SequenceEqual(foreignKey.DeclaringEntityType.FindPrimaryKey().Properties))
+            if (foreignKey.IsBaseLinking())
             {
                 return DeleteBehavior.ClientCascade;
             }

src/EFCore/Extensions/ForeignKeyExtensions.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.Linq;
 using System.Text;
 using JetBrains.Annotations;
 using Microsoft.EntityFrameworkCore.ChangeTracking;
@@ -71,6 +72,18 @@ public static IEntityType GetRelatedEntityType([NotNull] this IForeignKey foreig
         public static INavigation GetNavigation([NotNull] this IForeignKey foreignKey, bool pointsToPrincipal)
             => pointsToPrincipal ? foreignKey.DependentToPrincipal : foreignKey.PrincipalToDependent;
 
+        /// <summary>
+        ///     Returns a value indicating whether the foreign key is defined on the primary key and pointing to the same primary key.
+        /// </summary>
+        /// <param name="foreignKey"> The foreign key. </param>
+        /// <returns> A value indicating whether the foreign key is defined on the primary key and pointing to the same primary key. </returns>
+        public static bool IsBaseLinking([NotNull] this IForeignKey foreignKey)
+        {
+            var primaryKey = foreignKey.DeclaringEntityType.FindPrimaryKey();
+            return primaryKey == foreignKey.PrincipalKey
+                && foreignKey.Properties.SequenceEqual(primaryKey.Properties);
+        }
+
         /// <summary>
         ///     <para>
         ///         Creates a human-readable representation of the given metadata.

src/EFCore/Infrastructure/ModelValidator.cs

@@ -776,9 +776,7 @@ private bool Contains(IForeignKey inheritedFk, IForeignKey derivedFk)
                     }
 
                     if (entityType.BaseType == null
-                        || (declaredForeignKey.PrincipalEntityType.IsAssignableFrom(entityType)
-                            && declaredForeignKey.PrincipalKey.IsPrimaryKey()
-                            && declaredForeignKey.PrincipalKey.Properties.SequenceEqual(declaredForeignKey.Properties)))
+                        || declaredForeignKey.IsBaseLinking())
                     {
                         continue;
                     }

src/EFCore/Metadata/Conventions/ForeignKeyPropertyDiscoveryConvention.cs

@@ -158,9 +158,8 @@ public ForeignKeyPropertyDiscoveryConvention([NotNull] ProviderConventionSetBuil
                 || (!foreignKey.IsUnique && !ConfigurationSource.Convention.Overrides(foreignKey.GetIsUniqueConfigurationSource()))
                 || foreignKey.PrincipalToDependent?.IsCollection == true
                 || foreignKey.DeclaringEntityType.FindOwnership() != null
-                || (foreignKey.PrincipalKey.IsPrimaryKey()
-                    && foreignKey.PrincipalEntityType.IsAssignableFrom(foreignKey.DeclaringEntityType)
-                    && foreignKey.Properties.SequenceEqual(foreignKey.DeclaringEntityType.FindPrimaryKey().Properties)))
+                || (foreignKey.IsBaseLinking()
+                    && foreignKey.PrincipalEntityType.IsAssignableFrom(foreignKey.DeclaringEntityType)))
             {
                 relationshipBuilder = relationshipBuilder.HasEntityTypes(
                     foreignKey.PrincipalEntityType, foreignKey.DeclaringEntityType);

src/EFCore/Metadata/Conventions/ValueGenerationConvention.cs

@@ -45,7 +45,7 @@ public ValueGenerationConvention([NotNull] ProviderConventionSetBuilderDependenc
             IConventionForeignKeyBuilder relationshipBuilder, IConventionContext<IConventionForeignKeyBuilder> context)
         {
             var foreignKey = relationshipBuilder.Metadata;
-            if (IsNonReduntant(foreignKey))
+            if (!foreignKey.IsBaseLinking())
             {
                 foreach (var property in foreignKey.Properties)
                 {
@@ -87,7 +87,7 @@ public ValueGenerationConvention([NotNull] ProviderConventionSetBuilderDependenc
                 OnForeignKeyRemoved(oldDependentProperties);
 
                 if (relationshipBuilder.Metadata.Builder != null
-                    && IsNonReduntant(foreignKey))
+                    && !foreignKey.IsBaseLinking())
                 {
                     foreach (var property in foreignKey.Properties)
                     {
@@ -184,7 +184,7 @@ private void OnForeignKeyRemoved(IReadOnlyList<IConventionProperty> foreignKeyPr
         /// <param name="property"> The property. </param>
         /// <returns> The store value generation strategy to set for the given property. </returns>
         public static ValueGenerated? GetValueGenerated([NotNull] IProperty property)
-            => !property.GetContainingForeignKeys().Any(IsNonReduntant)
+            => !property.GetContainingForeignKeys().Any(fk => !fk.IsBaseLinking())
                 && ShouldHaveGeneratedProperty(property.FindContainingPrimaryKey())
                 && CanBeGenerated(property)
                     ? ValueGenerated.OnAdd
@@ -225,10 +225,5 @@ private static bool CanBeGenerated(IProperty property)
                 property.Builder.ValueGenerated(GetValueGenerated(property));
             }
         }
-
-        private static bool IsNonReduntant(IForeignKey foreignKey)
-            => !(foreignKey.PrincipalKey.IsPrimaryKey()
-                && foreignKey.PrincipalEntityType.IsAssignableFrom(foreignKey.DeclaringEntityType)
-                && foreignKey.Properties.SequenceEqual(foreignKey.PrincipalKey.Properties));
     }
 }

src/EFCore/Metadata/Internal/ForeignKeyExtensions.cs

@@ -27,15 +27,6 @@ public static class ForeignKeyExtensions
         public static bool IsSelfReferencing([NotNull] this IForeignKey foreignKey)
             => foreignKey.DeclaringEntityType == foreignKey.PrincipalEntityType;
 
-        /// <summary>
-        ///     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.
-        /// </summary>
-        public static bool IsSelfPrimaryKeyReferencing([NotNull] this IForeignKey foreignKey)
-            => foreignKey.DeclaringEntityType.FindPrimaryKey() == foreignKey.PrincipalKey;
-
         /// <summary>
         ///     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

test/EFCore.SqlServer.FunctionalTests/Query/TPTInheritanceQuerySqlServerTest.cs

@@ -2,8 +2,6 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System.Threading.Tasks;
-using Microsoft.EntityFrameworkCore.Infrastructure;
-using Microsoft.EntityFrameworkCore.Storage;
 using Xunit.Abstractions;
 
 // ReSharper disable InconsistentNaming
@@ -111,20 +109,20 @@ public override void Can_insert_update_delete()
 INSERT INTO [Animals] ([Species], [CountryId], [Name])
 VALUES (@p0, @p1, @p2);",
                 //
-                @"@p0='Apteryx owenii' (Nullable = false) (Size = 100)
-@p1=NULL (Size = 100)
-@p2='True'
+                @"@p3='Apteryx owenii' (Nullable = false) (Size = 100)
+@p4=NULL (Size = 100)
+@p5='True'
 
 SET NOCOUNT ON;
 INSERT INTO [Birds] ([Species], [EagleId], [IsFlightless])
-VALUES (@p0, @p1, @p2);",
+VALUES (@p3, @p4, @p5);",
                 //
-                @"@p0='Apteryx owenii' (Nullable = false) (Size = 100)
-@p1='0' (Size = 1)
+                @"@p6='Apteryx owenii' (Nullable = false) (Size = 100)
+@p7='0' (Size = 1)
 
 SET NOCOUNT ON;
 INSERT INTO [Kiwi] ([Species], [FoundOn])
-VALUES (@p0, @p1);",
+VALUES (@p6, @p7);",
                 //
                 @"SELECT TOP(2) [a].[Species], [a].[CountryId], [a].[Name], [b].[EagleId], [b].[IsFlightless], [k].[FoundOn]
 FROM [Animals] AS [a]
@@ -149,22 +147,22 @@ public override void Can_insert_update_delete()
                 @"@p0='Apteryx owenii' (Nullable = false) (Size = 100)
 
 SET NOCOUNT ON;
-DELETE FROM [Birds]
+DELETE FROM [Kiwi]
 WHERE [Species] = @p0;
 SELECT @@ROWCOUNT;",
                 //
-                @"@p0='Apteryx owenii' (Nullable = false) (Size = 100)
+                @"@p1='Apteryx owenii' (Nullable = false) (Size = 100)
 
 SET NOCOUNT ON;
-DELETE FROM [Kiwi]
-WHERE [Species] = @p0;
+DELETE FROM [Birds]
+WHERE [Species] = @p1;
 SELECT @@ROWCOUNT;",
                 //
-                @"@p1='Apteryx owenii' (Nullable = false) (Size = 100)
+                @"@p2='Apteryx owenii' (Nullable = false) (Size = 100)
 
 SET NOCOUNT ON;
 DELETE FROM [Animals]
-WHERE [Species] = @p1;
+WHERE [Species] = @p2;
 SELECT @@ROWCOUNT;",
                 //
                 @"SELECT COUNT(*)

test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs

@@ -883,15 +883,16 @@ public virtual void Detects_owned_entity_type_without_ownership()
         public virtual void Detects_ForeignKey_on_inherited_generated_key_property()
         {
             var modelBuilder = CreateConventionalModelBuilder();
-            modelBuilder.Entity<Abstract>().Property(e => e.Id).ValueGeneratedOnAdd();
-            modelBuilder.Entity<Generic<int>>().HasOne<Abstract>().WithOne().HasForeignKey<Generic<int>>(e => e.Id);
+            modelBuilder.Entity<Abstract>().Property<int>("SomeId").ValueGeneratedOnAdd();
+            modelBuilder.Entity<Abstract>().HasAlternateKey("SomeId");
+            modelBuilder.Entity<Generic<int>>().HasOne<Abstract>().WithOne().HasForeignKey<Generic<int>>("SomeId");
             modelBuilder.Entity<Generic<string>>();
 
             VerifyError(
                 CoreStrings.ForeignKeyPropertyInKey(
-                    nameof(Abstract.Id),
+                    "SomeId",
                     "Generic<int>",
-                    "{'" + nameof(Abstract.Id) + "'}",
+                    "{'SomeId'}",
                     nameof(Abstract)), modelBuilder.Model);
         }
 

test/EFCore.Tests/Metadata/Conventions/ValueGeneratorConventionTest.cs

@@ -447,29 +447,23 @@ public void Identity_is_added_when_foreign_key_is_removed_and_key_is_primary_key
         #endregion
 
         private static void RunConvention(InternalEntityTypeBuilder entityBuilder)
-        {
-            new ValueGenerationConvention(CreateDependencies())
+            => new ValueGenerationConvention(CreateDependencies())
                 .ProcessEntityTypePrimaryKeyChanged(
                     entityBuilder, entityBuilder.Metadata.FindPrimaryKey(), null,
                     new ConventionContext<IConventionKey>(entityBuilder.Metadata.Model.ConventionDispatcher));
-        }
 
         private static void RunConvention(InternalForeignKeyBuilder foreignKeyBuilder)
-        {
-            new ValueGenerationConvention(CreateDependencies())
+            => new ValueGenerationConvention(CreateDependencies())
                 .ProcessForeignKeyAdded(
                     foreignKeyBuilder,
                     new ConventionContext<IConventionForeignKeyBuilder>(
                         foreignKeyBuilder.Metadata.DeclaringEntityType.Model.ConventionDispatcher));
-        }
 
         private static void RunConvention(InternalEntityTypeBuilder entityBuilder, ForeignKey foreignKey)
-        {
-            new ValueGenerationConvention(CreateDependencies())
+            => new ValueGenerationConvention(CreateDependencies())
                 .ProcessForeignKeyRemoved(
                     entityBuilder, foreignKey,
                     new ConventionContext<IConventionForeignKey>(entityBuilder.Metadata.Model.ConventionDispatcher));
-        }
 
         private static ProviderConventionSetBuilderDependencies CreateDependencies()
             => InMemoryTestHelpers.Instance.CreateContextServices().GetRequiredService<ProviderConventionSetBuilderDependencies>();

test/EFCore.Tests/Metadata/Internal/ForeignKeyTest.cs

@@ -282,8 +282,7 @@ private IMutableForeignKey CreateOneToManySameBaseFK()
 
             var dependentEntityType = model.AddEntityType(typeof(OneToManyDependent));
             dependentEntityType.BaseType = baseEntityType;
-            var fkProp = dependentEntityType.AddProperty("Fk", typeof(int));
-            var fk = dependentEntityType.AddForeignKey(new[] { fkProp }, pk, principalEntityType);
+            var fk = dependentEntityType.AddForeignKey(new[] { property1 }, pk, principalEntityType);
             fk.SetPrincipalToDependent(NavigationBase.OneToManyDependentsProperty);
             fk.SetDependentToPrincipal(NavigationBase.OneToManyPrincipalProperty);
             return fk;
@@ -299,8 +298,7 @@ private IMutableForeignKey CreateOneToManySameHierarchyFK()
 
             var dependentEntityType = model.AddEntityType(typeof(OneToManyDependent));
             dependentEntityType.BaseType = baseEntityType;
-            var fkProp = dependentEntityType.AddProperty("Fk", typeof(int));
-            var fk = dependentEntityType.AddForeignKey(new[] { fkProp }, pk, baseEntityType);
+            var fk = dependentEntityType.AddForeignKey(new[] { property1 }, pk, baseEntityType);
             fk.SetPrincipalToDependent(NavigationBase.OneToManyDependentsProperty);
             return fk;
         }
@@ -417,14 +415,13 @@ private IMutableForeignKey CreateSelfRefFK(bool useAltKey = false)
         {
             var entityType = CreateModel().AddEntityType(typeof(SelfRef));
             var pk = entityType.SetPrimaryKey(entityType.AddProperty(SelfRef.IdProperty));
-            var fkProp = entityType.AddProperty(SelfRef.SelfRefIdProperty);
 
             var property = entityType.AddProperty("AltId", typeof(int));
             var principalKey = useAltKey
                 ? entityType.AddKey(property)
                 : pk;
 
-            var fk = entityType.AddForeignKey(new[] { fkProp }, principalKey, entityType);
+            var fk = entityType.AddForeignKey(new[] { pk.Properties.Single() }, principalKey, entityType);
             fk.IsUnique = true;
             fk.SetDependentToPrincipal(SelfRef.SelfRefPrincipalProperty);
             fk.SetPrincipalToDependent(SelfRef.SelfRefDependentProperty);
@@ -485,43 +482,43 @@ public void IsSelfReferencing_returns_false_for_non_hierarchical_foreign_keys()
         }
 
         [ConditionalFact]
-        public void IsSelfPrimaryKeyReferencing_returns_true_for_self_ref_foreign_keys()
+        public void IsBaseLinking_returns_true_for_self_ref_foreign_keys()
         {
             var fk = CreateSelfRefFK();
 
-            Assert.True(fk.IsSelfPrimaryKeyReferencing());
+            Assert.True(fk.IsBaseLinking());
         }
 
         [ConditionalFact]
-        public void IsSelfPrimaryKeyReferencing_returns_false_for_non_pk_self_ref_foreign_keys()
+        public void IsBaseLinking_returns_false_for_non_pk_self_ref_foreign_keys()
         {
             var fk = CreateSelfRefFK(useAltKey: true);
 
-            Assert.False(fk.IsSelfPrimaryKeyReferencing());
+            Assert.False(fk.IsBaseLinking());
         }
 
         [ConditionalFact]
-        public void IsSelfPrimaryKeyReferencing_returns_true_for_same_hierarchy_foreign_keys()
+        public void IsBaseLinking_returns_true_for_same_hierarchy_foreign_keys()
         {
             var fk = CreateOneToManySameHierarchyFK();
 
-            Assert.True(fk.IsSelfPrimaryKeyReferencing());
+            Assert.True(fk.IsBaseLinking());
         }
 
         [ConditionalFact]
-        public void IsSelfPrimaryKeyReferencing_returns_true_for_same_base_foreign_keys()
+        public void IsBaseLinking_returns_true_for_same_base_foreign_keys()
         {
             var fk = CreateOneToManySameBaseFK();
 
-            Assert.True(fk.IsSelfPrimaryKeyReferencing());
+            Assert.True(fk.IsBaseLinking());
         }
 
         [ConditionalFact]
         public void IsSelfPrimaryKeyReferencing_returns_false_for_non_hierarchical_foreign_keys()
         {
             var fk = CreateOneToManyFK();
 
-            Assert.False(fk.IsSelfPrimaryKeyReferencing());
+            Assert.False(fk.IsBaseLinking());
         }
 
         [ConditionalFact]