Generate temp values for key properties contained in optional FKs

Don't throw for unknown values that won't be sent to the database Fixes #27455
dotnet · Mar 6, 2022 · 29b62d8 · 29b62d8
1 parent bd15f9d
commit 29b62d8
Show file tree

Hide file tree

Showing 4 changed files with 79 additions and 6 deletions.
diff --git a/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs b/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs
@@ -1459,6 +1459,9 @@ public void AcceptChanges()
             }
         }
 
+        private readonly static bool _useOldBehavior27455 =
+            AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue27455", out var enabled27455) && enabled27455;
+
         /// <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
@@ -1485,7 +1488,8 @@ public InternalEntityEntry PrepareToSave()
 
                     if (property.IsKey()
                         && property.IsForeignKey()
-                        && _stateData.IsPropertyFlagged(property.GetIndex(), PropertyFlag.Unknown))
+                        && _stateData.IsPropertyFlagged(property.GetIndex(), PropertyFlag.Unknown)
+                        && (_useOldBehavior27455 || !IsStoreGenerated(property)))
                     {
                         throw new InvalidOperationException(CoreStrings.UnknownKeyValue(entityType.DisplayName(), property.Name));
                     }

diff --git a/src/EFCore/Metadata/Internal/PropertyExtensions.cs b/src/EFCore/Metadata/Internal/PropertyExtensions.cs
@@ -1,7 +1,9 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System;
 using System.Collections.Generic;
+using System.Linq;
 using Microsoft.EntityFrameworkCore.Infrastructure;
 using Microsoft.EntityFrameworkCore.Utilities;
 
@@ -74,6 +76,9 @@ public static bool ForUpdate(this ValueGenerated valueGenerated)
             return null;
         }
 
+        private readonly static bool _useOldBehavior27455 =
+            AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue27455", out var enabled27455) && enabled27455;
+
         /// <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
@@ -83,7 +88,10 @@ public static bool ForUpdate(this ValueGenerated valueGenerated)
         public static bool RequiresValueGenerator(this IReadOnlyProperty property)
             => (property.ValueGenerated.ForAdd()
                     && property.IsKey()
-                    && (!property.IsForeignKey() || property.IsForeignKeyToSelf()))
+                    && (!property.IsForeignKey()
+                        || property.IsForeignKeyToSelf()
+                        || (!_useOldBehavior27455
+                            && property.GetContainingForeignKeys().All(fk => fk.Properties.Any(p => p != property && p.IsNullable)))))
                 || property.GetValueGeneratorFactory() != null;
 
         /// <summary>

diff --git a/test/EFCore.Specification.Tests/StoreGeneratedTestBase.cs b/test/EFCore.Specification.Tests/StoreGeneratedTestBase.cs
@@ -512,6 +512,45 @@ public virtual void Identity_property_on_Added_entity_with_temporary_value_gets_
                 context => Assert.Equal("Banana Joe", context.Set<Gumball>().Single(e => e.Id == id).Identity));
         }
 
+#nullable enable
+        protected class CompositePrincipal
+        {
+            public int Id { get; set; }
+            public int? CurrentNumber { get; set; }
+            public CompositeDependent? Current { get; set; }
+            public ICollection<CompositeDependent> Periods { get; } = new HashSet<CompositeDependent>();
+        }
+
+        protected class CompositeDependent
+        {
+            public int PrincipalId { get; set; }
+            public int Number { get; set; }
+            public CompositePrincipal? Principal { get; set; }
+        }
+#nullable disable
+
+        [ConditionalFact]
+        public virtual void Store_generated_values_are_propagated_with_composite_key_cycles()
+        {
+            var id = 0;
+
+            ExecuteWithStrategyInTransaction(
+                context =>
+                {
+                    var period = new CompositeDependent
+                    {
+                        Number = 1,
+                        Principal = new CompositePrincipal()
+                    };
+
+                    context.Add(period);
+                    context.SaveChanges();
+
+                    id = period.PrincipalId;
+                },
+                context => Assert.Equal(1, context.Set<CompositeDependent>().Single(e => e.PrincipalId == id).Number));
+        }
+
         protected class NonStoreGenDependent
         {
             [DatabaseGenerated(DatabaseGeneratedOption.None)]
@@ -1947,10 +1986,30 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
                 modelBuilder.Entity<OptionalProduct>();
                 modelBuilder.Entity<StoreGenPrincipal>();
 
-                modelBuilder.Entity<NonStoreGenDependent>()
-                    .Property(e => e.HasTemp)
-                    .ValueGeneratedOnAddOrUpdate()
-                    .HasValueGenerator<TemporaryIntValueGenerator>();
+                modelBuilder.Entity<NonStoreGenDependent>(eb =>
+                {
+                    eb.Property(e => e.HasTemp)
+                        .ValueGeneratedOnAddOrUpdate()
+                        .HasValueGenerator<TemporaryIntValueGenerator>();
+                });
+
+                modelBuilder.Entity<CompositePrincipal>(entity =>
+                {
+                    entity.HasKey(x => x.Id);
+                    entity.Property(x => x.Id)
+                        .ValueGeneratedOnAdd();
+                    entity.HasOne(x => x.Current)
+                        .WithOne()
+                        .HasForeignKey<CompositePrincipal>(x => new { x.Id, x.CurrentNumber });
+                });
+
+                modelBuilder.Entity<CompositeDependent>(entity =>
+                {
+                    entity.HasKey(x => new { x.PrincipalId, x.Number });
+                    entity.HasOne(x => x.Principal)
+                        .WithMany(x => x.Periods)
+                        .HasForeignKey(x => x.PrincipalId);
+                });
             }
         }
     }

diff --git a/test/EFCore.SqlServer.FunctionalTests/StoreGeneratedSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/StoreGeneratedSqlServerTest.cs
@@ -228,6 +228,8 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
 
                 modelBuilder.Entity<NonStoreGenDependent>().Property(e => e.HasTemp).HasDefaultValue(777);
 
+                modelBuilder.Entity<CompositePrincipal>().Property(e => e.Id).UseIdentityColumn();
+
                 base.OnModelCreating(modelBuilder, context);
             }
         }
-Original file line number
+Diff line change
@@ Expand Up @@
                     modelBuilder.Entity<NonStoreGenDependent>().Property(e => e.HasTemp).HasDefaultValue(777);
+                    modelBuilder.Entity<CompositePrincipal>().Property(e => e.Id).UseIdentityColumn();
                     base.OnModelCreating(modelBuilder, context);
                 }
             }
@@ Expand Down @@