Remove temporary properties created by ForeignKeyPropertyDiscoveryCon…

…vention if they aren't used. (#33611) Fixes #33531
dotnet · Apr 30, 2024 · b154f2d · b154f2d
1 parent ebbc5ee
commit b154f2d
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 6 deletions.
diff --git a/src/EFCore/Metadata/Conventions/ForeignKeyPropertyDiscoveryConvention.cs b/src/EFCore/Metadata/Conventions/ForeignKeyPropertyDiscoveryConvention.cs
@@ -134,6 +134,7 @@ public ForeignKeyPropertyDiscoveryConvention(ProviderConventionSetBuilderDepende
                         || !foreignKey.Properties.SequenceEqual(foreignKeyProperties)))))
         {
             var batch = context.DelayConventions();
+            var newProperties = new List<IConventionProperty?>();
             using var foreignKeyReference = batch.Track(foreignKey);
             foreach (var fkProperty in foreignKey.Properties)
             {
@@ -145,15 +146,23 @@ public ForeignKeyPropertyDiscoveryConvention(ProviderConventionSetBuilderDepende
                     var newType = fkProperty.ClrType.MakeNullable(!foreignKey.IsRequired);
                     if (fkProperty.ClrType != newType)
                     {
-                        fkProperty.DeclaringType.Builder.Property(
-                            newType,
-                            fkProperty.Name,
-                            fkProperty.GetConfigurationSource() == ConfigurationSource.DataAnnotation);
+                        newProperties.Add(
+                            fkProperty.DeclaringType.Builder.Property(
+                                newType,
+                                fkProperty.Name,
+                                fkProperty.GetConfigurationSource() == ConfigurationSource.DataAnnotation)?.Metadata);
                     }
                 }
             }
 
             batch.Dispose();
+
+            // If the new properties didn't end up being used we need to remove them
+            foreach (var newProperty in newProperties)
+            {
+                newProperty?.DeclaringType.Builder.RemoveUnusedImplicitProperties([newProperty]);
+            }
+
             return foreignKeyReference.Object is null || !foreignKeyReference.Object.IsInModel
                 ? null
                 : foreignKeyReference.Object.Builder;

diff --git a/test/EFCore.Specification.Tests/ModelBuilding101OneToManyTestBase.cs b/test/EFCore.Specification.Tests/ModelBuilding101OneToManyTestBase.cs
@@ -1369,6 +1369,19 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
                     .WithOne(e => e.Blog)
                     .HasPrincipalKey(e => e.AlternateId);
         }
+
+        public class ContextAnnotated1 : ContextAnnotated0
+        {
+            protected override void OnModelCreating(ModelBuilder modelBuilder)
+            {
+                base.OnModelCreating(modelBuilder);
+
+                modelBuilder.Entity<Post>().HasIndex(p => p.BlogId);
+            }
+
+            protected override void ConfigureConventions(ModelConfigurationBuilder configurationBuilder)
+                => configurationBuilder.Conventions.Remove<ForeignKeyIndexConvention>();
+        }
     }
 
     [ConditionalFact]

diff --git a/test/EFCore.Specification.Tests/ModelBuilding101OneToOneNrtTestBase.cs b/test/EFCore.Specification.Tests/ModelBuilding101OneToOneNrtTestBase.cs
@@ -1272,7 +1272,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
 
     [ConditionalFact]
     public virtual void OneToOneRequiredWithAlternateKeyNrtTest()
-        => Assert.Throws<EqualException>(() => Model101Test()); // Issue #30346
+        => Model101Test();
 
     protected class OneToOneRequiredWithAlternateKeyNrt
     {

diff --git a/test/EFCore.Specification.Tests/ModelBuilding101OneToOneTestBase.cs b/test/EFCore.Specification.Tests/ModelBuilding101OneToOneTestBase.cs
@@ -1279,7 +1279,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
 
     [ConditionalFact]
     public virtual void OneToOneRequiredWithAlternateKeyTest()
-        => Assert.Throws<EqualException>(() => Model101Test()); // Issue #30346
+        => Model101Test();
 
     protected class OneToOneRequiredWithAlternateKey
     {