Avoid stack overflow in negative many-to-many cases

Fixes #23377 Fixes #23354
dotnet · Nov 24, 2020 · 314a09a · 314a09a
1 parent 240223e
commit 314a09a
Show file tree

Hide file tree

Showing 6 changed files with 58 additions and 11 deletions.
diff --git a/src/EFCore.Relational/Extensions/RelationalEntityTypeExtensions.cs b/src/EFCore.Relational/Extensions/RelationalEntityTypeExtensions.cs
@@ -153,12 +153,12 @@ public static string GetDefaultSchema([NotNull] this IEntityType entityType)
             }
             else
             {
-                var skipNavigationSchema = entityType.GetForeignKeys().SelectMany(fk => fk.GetReferencingSkipNavigations())
-                    .FirstOrDefault(n => !n.IsOnDependent)
-                    ?.DeclaringEntityType.GetSchema();
+                var skipReferencingTypes = entityType.GetForeignKeys().SelectMany(fk => fk.GetReferencingSkipNavigations())
+                    .Where(n => !n.IsOnDependent && n.DeclaringEntityType != entityType)
+                    .ToList();
+                var skipNavigationSchema = skipReferencingTypes.FirstOrDefault()?.DeclaringEntityType.GetSchema();
                 if (skipNavigationSchema != null
-                    && entityType.GetForeignKeys().SelectMany(fk => fk.GetReferencingSkipNavigations())
-                        .Where(n => !n.IsOnDependent)
+                    && skipReferencingTypes.Skip(1)
                         .All(n => n.DeclaringEntityType.GetSchema() == skipNavigationSchema))
                 {
                     return skipNavigationSchema;

diff --git a/src/EFCore.SqlServer/Metadata/Conventions/SqlServerOnDeleteConvention.cs b/src/EFCore.SqlServer/Metadata/Conventions/SqlServerOnDeleteConvention.cs
@@ -60,7 +60,8 @@ protected override DeleteBehavior GetTargetDeleteBehavior(IConventionForeignKey
 
             if (selfReferencingSkipNavigation
                 == selfReferencingSkipNavigation.DeclaringEntityType.GetDeclaredSkipNavigations()
-                    .First(s => s == selfReferencingSkipNavigation || s == selfReferencingSkipNavigation.Inverse))
+                    .First(s => s == selfReferencingSkipNavigation || s == selfReferencingSkipNavigation.Inverse)
+                && selfReferencingSkipNavigation != selfReferencingSkipNavigation.Inverse)
             {
                 selfReferencingSkipNavigation.Inverse.ForeignKey?.Builder.OnDelete(
                     GetTargetDeleteBehavior(selfReferencingSkipNavigation.Inverse.ForeignKey));

diff --git a/src/EFCore/Metadata/Conventions/ForeignKeyPropertyDiscoveryConvention.cs b/src/EFCore/Metadata/Conventions/ForeignKeyPropertyDiscoveryConvention.cs
@@ -721,11 +721,14 @@ public virtual void ProcessKeyRemoved(
                 .SelectMany(t => t.GetDeclaredForeignKeys()).ToList();
             foreach (var foreignKey in foreignKeys)
             {
-                if ((!foreignKey.IsUnique
-                    || foreignKey.DeclaringEntityType.BaseType != null))
+                if ((foreignKey.IsUnique
+                    && foreignKey.DeclaringEntityType.BaseType == null)
+                    || foreignKey.Builder == null)
                 {
-                    DiscoverProperties(foreignKey.Builder, context);
+                    continue;
                 }
+
+                DiscoverProperties(foreignKey.Builder, context);
             }
         }
 

diff --git a/src/EFCore/Metadata/Conventions/KeyDiscoveryConvention.cs b/src/EFCore/Metadata/Conventions/KeyDiscoveryConvention.cs
@@ -124,7 +124,8 @@ protected virtual void TryConfigurePrimaryKey([NotNull] IConventionEntityTypeBui
             {
                 var manyToManyForeignKeys = entityType.GetForeignKeys()
                     .Where(fk => fk.GetReferencingSkipNavigations().Any(n => n.IsCollection)).ToList();
-                if (manyToManyForeignKeys.Count == 2)
+                if (manyToManyForeignKeys.Count == 2
+                    && !manyToManyForeignKeys.Any(fk => fk.PrincipalEntityType == entityType))
                 {
                     keyProperties.AddRange(manyToManyForeignKeys.SelectMany(fk => fk.Properties));
                 }

diff --git a/test/EFCore.Tests/ModelBuilding/ManyToManyTestBase.cs b/test/EFCore.Tests/ModelBuilding/ManyToManyTestBase.cs
@@ -343,6 +343,35 @@ public virtual void Throws_for_many_to_many_with_only_one_navigation_configured(
                             .WithMany(d => d.ManyToManyPrincipals)).Message);
             }
 
+            [ConditionalFact]
+            public virtual void Throws_for_self_ref_with_same_navigation()
+            {
+                var modelBuilder = CreateModelBuilder();
+
+                modelBuilder.Entity<SelfRefManyToOne>().Ignore(s => s.SelfRef1);
+                modelBuilder.Entity<SelfRefManyToOne>().HasMany(t => t.SelfRef2)
+                    .WithMany(t => t.SelfRef2);
+
+                Assert.Equal(CoreStrings.EntityRequiresKey("SelfRefManyToOneSelfRefManyToOne"),
+                    Assert.Throws<InvalidOperationException>(() => modelBuilder.FinalizeModel()).Message);
+            }
+
+            [ConditionalFact]
+            public virtual void Throws_for_self_ref_using_self()
+            {
+                var modelBuilder = CreateModelBuilder();
+
+                modelBuilder.Entity<SelfRefManyToOne>().Ignore(s => s.Id);
+                modelBuilder.Entity<SelfRefManyToOne>().HasMany(t => t.Relateds)
+                    .WithMany(t => t.RelatedSelfRefs)
+                    .UsingEntity<SelfRefManyToOne>(
+                    t => t.HasOne(a => a.Related).WithMany(b => b.DirectlyRelatedSelfRefs),
+                    t => t.HasOne(a => a.SelfRef1).WithMany(b => b.SelfRef2));
+
+                Assert.Equal(CoreStrings.EntityRequiresKey(nameof(SelfRefManyToOne)),
+                    Assert.Throws<InvalidOperationException>(() => modelBuilder.FinalizeModel()).Message);
+            }
+
             [ConditionalFact]
             public virtual void Throws_for_ForeignKeyAttribute_on_navigation()
             {

diff --git a/test/EFCore.Tests/ModelBuilding/TestModel.cs b/test/EFCore.Tests/ModelBuilding/TestModel.cs
@@ -317,9 +317,22 @@ protected class SelfRef
         protected class SelfRefManyToOne
         {
             public int Id { get; set; }
+            public int SelfRefId { get; set; }
             public SelfRefManyToOne SelfRef1 { get; set; }
             public ICollection<SelfRefManyToOne> SelfRef2 { get; set; }
-            public int SelfRefId { get; set; }
+
+            [NotMapped]
+            public ManyToManyRelated Related { get; set; }
+
+            [NotMapped]
+            public ICollection<ManyToManyRelated> Relateds { get; set; }
+        }
+
+        protected class ManyToManyRelated
+        {
+            public int Id { get; set; }
+            public ICollection<SelfRefManyToOne> DirectlyRelatedSelfRefs { get; set; }
+            public ICollection<SelfRefManyToOne> RelatedSelfRefs { get; set; }
         }
 
         protected class User