[release/8.0] Revert behavior to throw when attempting to modify an u…

…nconstrained alternate key property (#32523)

* Revert behavior to throw when attempting to modify an unconstrained alternate key property (#32492)

Fixes #28961
Reverts #30213 for #32385

In #32385, an unconstrained alternate key was added to the model purely to make a non-identifying relationship artificially identifying. #30213 attempted to fix this by throwing that the key was being modified. However, this scenario is very similar to the case for a many-to-many join type, where the composite primary key is also not the end of any relationship, but forces the two many-to-one relationships to be identifying.

I prepared a PR that would only throw if the key involved is alternate, but on reflection that doesn't seem like an appropriate distinction to make. So overall, I think we should just revert this change, which is what this PR does.

* Quirk

src/EFCore/ChangeTracking/Internal/NavigationFixer.cs

@@ -14,6 +14,9 @@ namespace Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
 /// </summary>
 public class NavigationFixer : INavigationFixer
 {
+    private static readonly bool UseOldBehavior32383 =
+        AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue32383", out var enabled32383) && enabled32383;
+
     private IList<(
         InternalEntityEntry Entry,
         InternalEntityEntry OtherEntry,
@@ -1412,14 +1415,51 @@ private void ConditionallyNullForeignKeyProperties(
             }
         }
 
-        if (foreignKey.IsRequired
-            && hasOnlyKeyProperties
-            && dependentEntry.EntityState != EntityState.Detached
-            && dependentEntry.EntityState != EntityState.Deleted)
+        if (UseOldBehavior32383)
         {
-            if (foreignKey.DeleteBehavior == DeleteBehavior.Cascade
-                || foreignKey.DeleteBehavior == DeleteBehavior.ClientCascade
-                || foreignKey.IsOwnership)
+            if (foreignKey.IsRequired
+                && hasOnlyKeyProperties
+                && dependentEntry.EntityState != EntityState.Detached
+                && dependentEntry.EntityState != EntityState.Deleted)
+            {
+                if (foreignKey.DeleteBehavior == DeleteBehavior.Cascade
+                    || foreignKey.DeleteBehavior == DeleteBehavior.ClientCascade
+                    || foreignKey.IsOwnership)
+                {
+                    try
+                    {
+                        _inFixup = true;
+                        switch (dependentEntry.EntityState)
+                        {
+                            case EntityState.Added:
+                                dependentEntry.SetEntityState(EntityState.Detached);
+                                DeleteFixup(dependentEntry);
+                                break;
+                            case EntityState.Unchanged:
+                            case EntityState.Modified:
+                                dependentEntry.SetEntityState(
+                                    dependentEntry.SharedIdentityEntry != null ? EntityState.Detached : EntityState.Deleted);
+                                DeleteFixup(dependentEntry);
+                                break;
+                        }
+                    }
+                    finally
+                    {
+                        _inFixup = false;
+                    }
+                }
+                else
+                {
+                    throw new InvalidOperationException(
+                        CoreStrings.KeyReadOnly(dependentProperties.First().Name, dependentEntry.EntityType.DisplayName()));
+                }
+            }
+        }
+        else
+        {
+            if (foreignKey.IsRequired
+                && hasOnlyKeyProperties
+                && dependentEntry.EntityState != EntityState.Detached)
             {
                 try
                 {
@@ -1443,11 +1483,7 @@ private void ConditionallyNullForeignKeyProperties(
                     _inFixup = false;
                 }
             }
-            else
-            {
-                throw new InvalidOperationException(
-                    CoreStrings.KeyReadOnly(dependentProperties.First().Name, dependentEntry.EntityType.DisplayName()));
-            }
+
         }
     }
 

test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBaseMiscellaneous.cs

@@ -1962,7 +1962,7 @@ private static SecondLaw AddSecondLevel(bool thirdLevel1, bool thirdLevel2)
         return secondLevel;
     }
 
-    [ConditionalTheory] // Issue #28961
+    [ConditionalTheory] // Issue #28961 and Issue #32385
     [InlineData(false)]
     [InlineData(true)]
     public virtual Task Alternate_key_over_foreign_key_doesnt_bypass_delete_behavior(bool async)
@@ -1976,16 +1976,14 @@ public virtual Task Alternate_key_over_foreign_key_doesnt_bypass_delete_behavior
                     ? await context.SaveChangesAsync()
                     : context.SaveChanges();
 
-                Assert.Equal(
-                    CoreStrings.KeyReadOnly(nameof(SneakyChild.ParentId), nameof(SneakyChild)),
-                    (await Assert.ThrowsAsync<InvalidOperationException>(
-                        async () =>
-                        {
-                            parent.Children.Remove(parent.Children.First());
-                            _ = async
-                                ? await context.SaveChangesAsync()
-                                : context.SaveChanges();
-                        })).Message);
+                Assert.Equal(2, context.ChangeTracker.Entries().Count());
+
+                parent.Children.Remove(parent.Children.First());
+                _ = async
+                    ? await context.SaveChangesAsync()
+                    : context.SaveChanges();
+
+                Assert.Equal(1, context.ChangeTracker.Entries().Count());
             });
 
     [ConditionalTheory] // Issue #30764

test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBaseOneToOne.cs

@@ -856,9 +856,7 @@ public virtual void Save_required_one_to_one_changed_by_reference(
 
                 if (Fixture.ForceClientNoAction)
                 {
-                    Assert.Equal(
-                        CoreStrings.KeyReadOnly(nameof(RequiredSingle1.Id), nameof(RequiredSingle1)),
-                        Assert.Throws<InvalidOperationException>(() => context.SaveChanges()).Message);
+                    Assert.Throws<DbUpdateException>(() => context.SaveChanges());
                 }
                 else
                 {
@@ -1202,11 +1200,13 @@ public virtual void Sever_required_one_to_one(
                     throw new ArgumentOutOfRangeException(nameof(changeMechanism));
                 }
 
+                Assert.False(context.Entry(root).Reference(e => e.RequiredSingle).IsLoaded);
+                Assert.False(context.Entry(old1).Reference(e => e.Root).IsLoaded);
+                Assert.True(context.ChangeTracker.HasChanges());
+
                 if (Fixture.ForceClientNoAction)
                 {
-                    Assert.Equal(
-                        CoreStrings.KeyReadOnly(nameof(RequiredSingle1.Id), nameof(RequiredSingle1)),
-                        Assert.Throws<InvalidOperationException>(() => context.SaveChanges()).Message);
+                    Assert.Throws<DbUpdateException>(() => context.SaveChanges());
                 }
                 else
                 {