[release/9.0] Prevent owner entity from becoming optional (#35222)

Fixes #35110
dotnet · Dec 2, 2024 · cc53f41 · cc53f41
1 parent fba8789
commit cc53f41
Show file tree

Hide file tree

Showing 3 changed files with 78 additions and 15 deletions.
diff --git a/src/EFCore/Metadata/Conventions/ForeignKeyPropertyDiscoveryConvention.cs b/src/EFCore/Metadata/Conventions/ForeignKeyPropertyDiscoveryConvention.cs
@@ -50,6 +50,9 @@ public class ForeignKeyPropertyDiscoveryConvention :
     IPropertyFieldChangedConvention,
     IModelFinalizingConvention
 {
+    private static readonly bool UseOldBehavior35110 =
+        AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue35110", out var enabled) && enabled;
+
     /// <summary>
     ///     Creates a new instance of <see cref="ForeignKeyPropertyDiscoveryConvention" />.
     /// </summary>
@@ -81,13 +84,17 @@ private IConventionForeignKeyBuilder ProcessForeignKey(
         IConventionContext context)
     {
         var shouldBeRequired = true;
-        foreach (var property in relationshipBuilder.Metadata.Properties)
+        if (!relationshipBuilder.Metadata.IsOwnership
+            || UseOldBehavior35110)
         {
-            if (property.IsNullable)
+            foreach (var property in relationshipBuilder.Metadata.Properties)
             {
-                shouldBeRequired = false;
-                relationshipBuilder = relationshipBuilder.IsRequired(false) ?? relationshipBuilder;
-                break;
+                if (property.IsNullable)
+                {
+                    shouldBeRequired = false;
+                    relationshipBuilder = relationshipBuilder.IsRequired(false) ?? relationshipBuilder;
+                    break;
+                }
             }
         }
 

diff --git a/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs b/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs
@@ -15,6 +15,9 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Internal;
 /// </summary>
 public class InternalEntityTypeBuilder : InternalTypeBaseBuilder, IConventionEntityTypeBuilder
 {
+    private static readonly bool UseOldBehavior35110 =
+        AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue35110", out var enabled) && enabled;
+
     /// <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
@@ -3161,17 +3164,9 @@ public static InternalIndexBuilder DetachIndex(Index indexToDetach)
                         && targetEntityType.Name == existingTargetType.ClrType.DisplayName())))
             {
                 relationship = existingNavigation.ForeignKey.Builder;
-                if (existingNavigation.ForeignKey.IsOwnership)
-                {
-                    relationship = relationship.IsOwnership(true, configurationSource)
-                        ?.HasNavigations(inverse, navigation, configurationSource);
-
-                    relationship?.Metadata.UpdateConfigurationSource(configurationSource);
-                    return relationship;
-                }
-
                 Check.DebugAssert(
-                    !existingTargetType.IsOwned()
+                    existingNavigation.ForeignKey.IsOwnership
+                    || !existingTargetType.IsOwned()
                     || existingNavigation.DeclaringEntityType.IsInOwnershipPath(existingTargetType)
                     || (existingTargetType.IsInOwnershipPath(existingNavigation.DeclaringEntityType)
                         && existingTargetType.FindOwnership()!.PrincipalEntityType != existingNavigation.DeclaringEntityType),
@@ -3181,6 +3176,11 @@ public static InternalIndexBuilder DetachIndex(Index indexToDetach)
                 relationship = relationship.IsOwnership(true, configurationSource)
                     ?.HasNavigations(inverse, navigation, configurationSource);
 
+                if (!UseOldBehavior35110)
+                {
+                    relationship = relationship?.IsRequired(true, configurationSource);
+                }
+
                 relationship?.Metadata.UpdateConfigurationSource(configurationSource);
                 return relationship;
             }

diff --git a/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs b/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs
@@ -9763,6 +9763,62 @@ public void Update_AK_seed_value_with_a_referencing_foreign_key()
                         v => Assert.Equal(4242, v));
                 }));
 
+    [ConditionalFact]
+    public void Owned_collection_with_explicit_id()
+        => Execute(
+            modelBuilder =>
+            {
+            },
+            source =>
+            {
+                source.Entity("Microsoft.EntityFrameworkCore.Migrations.Internal.Account", b =>
+                {
+                    b.Property<string>("Id");
+                    b.HasKey("Id");
+                    b.ToTable("account");
+                });
+
+                source.Entity("Microsoft.EntityFrameworkCore.Migrations.Internal.Account", b =>
+                {
+                    b.OwnsMany("Microsoft.EntityFrameworkCore.Migrations.Internal.AccountHolder", "AccountHolders", b1 =>
+                    {
+                        b1.Property<string>("Id");
+                        b1.Property<string>("account_id");
+                        b1.HasKey("Id");
+                        b1.HasIndex("account_id");
+                        b1.ToTable("account_holder");
+                        b1.WithOwner().HasForeignKey("account_id");
+                    });
+                });
+            },
+            target =>
+            {
+                target.Entity<Account>(builder =>
+                {
+                    builder.ToTable("account");
+                    builder.HasKey("Id");
+                    builder.OwnsMany(a => a.AccountHolders, navigationBuilder =>
+                    {
+                        navigationBuilder.ToTable("account_holder");
+                        navigationBuilder.Property<string>("Id");
+                        navigationBuilder.HasKey("Id");
+                        navigationBuilder.Property<string>("account_id");
+                        navigationBuilder.WithOwner().HasForeignKey("account_id");
+                    });
+                });
+            },
+            Assert.Empty);
+
+    public class Account
+    {
+        public string Id { get; set; }
+        public IEnumerable<AccountHolder> AccountHolders { get; set; } = [];
+    }
+
+    public class AccountHolder
+    {
+    }
+
     [ConditionalFact]
     public void SeedData_with_guid_AK_and_multiple_owned_types()
         => Execute(