Allow database default values for FK properties that are part of a co…

…mposite key Fixes #27299
dotnet · Sep 12, 2022 · 01c6294 · 01c6294
1 parent 1362d96
commit 01c6294
Show file tree

Hide file tree

Showing 19 changed files with 444 additions and 107 deletions.
diff --git a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs
@@ -708,7 +708,7 @@ static bool IsNotNullAndFalse(object? value)
         {
             foreach (var key in entityType.GetDeclaredKeys())
             {
-                foreach (var property in key.Properties)
+                foreach (var property in key.Properties.Where(p => !p.IsForeignKey()))
                 {
                     var defaultValue = (IConventionAnnotation?)property.FindAnnotation(RelationalAnnotationNames.DefaultValue);
                     if (defaultValue?.Value != null
@@ -720,7 +720,7 @@ static bool IsNotNullAndFalse(object? value)
             }
         }
     }
-    
+
     /// <summary>
     ///     Validates the mapping/configuration of mutable in the model.
     /// </summary>

diff --git a/src/EFCore/ChangeTracking/Internal/IValueGenerationManager.cs b/src/EFCore/ChangeTracking/Internal/IValueGenerationManager.cs
@@ -33,6 +33,14 @@ public interface IValueGenerationManager
     /// </summary>
     InternalEntityEntry? Propagate(InternalEntityEntry entry);
 
+    /// <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
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    Task<InternalEntityEntry?> PropagateAsync(InternalEntityEntry entry, CancellationToken cancellationToken);
+
     /// <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

diff --git a/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs b/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs
@@ -186,23 +186,24 @@ void Setup()
     {
         var oldState = _stateData.EntityState;
         bool adding;
-        Setup();
+        await SetupAsync().ConfigureAwait(false);
 
         if ((adding || oldState is EntityState.Detached)
             && await StateManager.ValueGenerationManager
                 .GenerateAsync(this, includePrimaryKey: adding, cancellationToken).ConfigureAwait(false)
             && fallbackState.HasValue)
         {
             entityState = fallbackState.Value;
-            Setup();
+            await SetupAsync().ConfigureAwait(false);
         }
 
         SetEntityState(oldState, entityState, acceptChanges, modifyProperties);
 
-        void Setup()
+        async Task SetupAsync()
         {
             adding = PrepareForAdd(entityState);
-            entityState = PropagateToUnknownKey(oldState, entityState, adding, forceStateWhenUnknownKey);
+            entityState = await PropagateToUnknownKeyAsync(
+                oldState, entityState, adding, forceStateWhenUnknownKey, cancellationToken).ConfigureAwait(false);
         }
     }
 
@@ -214,27 +215,50 @@ void Setup()
     {
         var keyUnknown = IsKeyUnknown;
 
-        if (adding
-            || (oldState == EntityState.Detached
-                && keyUnknown))
+        if (adding || (oldState == EntityState.Detached && keyUnknown))
         {
             var principalEntry = StateManager.ValueGenerationManager.Propagate(this);
 
-            if (forceStateWhenUnknownKey.HasValue
-                && keyUnknown
-                && principalEntry != null
-                && principalEntry.EntityState != EntityState.Detached
-                && principalEntry.EntityState != EntityState.Deleted)
-            {
-                entityState = principalEntry.EntityState == EntityState.Added
-                    ? EntityState.Added
-                    : forceStateWhenUnknownKey.Value;
-            }
+            entityState = ForceState(entityState, forceStateWhenUnknownKey, keyUnknown, principalEntry);
         }
 
         return entityState;
     }
 
+    private async Task<EntityState> PropagateToUnknownKeyAsync(
+        EntityState oldState,
+        EntityState entityState,
+        bool adding,
+        EntityState? forceStateWhenUnknownKey,
+        CancellationToken cancellationToken)
+    {
+        var keyUnknown = IsKeyUnknown;
+
+        if (adding || (oldState == EntityState.Detached && keyUnknown))
+        {
+            var principalEntry = await StateManager.ValueGenerationManager.PropagateAsync(this, cancellationToken).ConfigureAwait(false);
+
+            entityState = ForceState(entityState, forceStateWhenUnknownKey, keyUnknown, principalEntry);
+        }
+
+        return entityState;
+    }
+
+    private static EntityState ForceState(
+        EntityState entityState,
+        EntityState? forceStateWhenUnknownKey,
+        bool keyUnknown,
+        InternalEntityEntry? principalEntry)
+        => forceStateWhenUnknownKey.HasValue
+            && keyUnknown
+            && principalEntry != null
+            && principalEntry.EntityState != EntityState.Detached
+            && principalEntry.EntityState != EntityState.Deleted
+                ? principalEntry.EntityState == EntityState.Added
+                    ? EntityState.Added
+                    : forceStateWhenUnknownKey.Value
+                : entityState;
+
     private bool PrepareForAdd(EntityState newState)
     {
         if (newState != EntityState.Added

diff --git a/src/EFCore/ChangeTracking/Internal/KeyPropagator.cs b/src/EFCore/ChangeTracking/Internal/KeyPropagator.cs
@@ -53,21 +53,7 @@ public class KeyPropagator : IKeyPropagator
 
             if (valueGenerator != null)
             {
-                var value = valueGenerator.Next(new EntityEntry(entry));
-
-                if (valueGenerator.GeneratesTemporaryValues)
-                {
-                    entry.SetTemporaryValue(property, value);
-                }
-                else
-                {
-                    entry[property] = value;
-                }
-
-                if (!valueGenerator.GeneratesStableValues)
-                {
-                    entry.MarkUnknown(property);
-                }
+                SetValue(entry, property, valueGenerator, valueGenerator.Next(new EntityEntry(entry)));
             }
         }
 
@@ -101,28 +87,30 @@ public class KeyPropagator : IKeyPropagator
 
             if (valueGenerator != null)
             {
-                var value = await valueGenerator.NextAsync(new EntityEntry(entry), cancellationToken)
-                    .ConfigureAwait(false);
-
-                if (valueGenerator.GeneratesTemporaryValues)
-                {
-                    entry.SetTemporaryValue(property, value);
-                }
-                else
-                {
-                    entry[property] = value;
-                }
-
-                if (!valueGenerator.GeneratesStableValues)
-                {
-                    entry.MarkUnknown(property);
-                }
+                SetValue(
+                    entry,
+                    property,
+                    valueGenerator,
+                    await valueGenerator.NextAsync(new EntityEntry(entry), cancellationToken).ConfigureAwait(false));
             }
         }
 
         return principalEntry;
     }
 
+    private static void SetValue(InternalEntityEntry entry, IProperty property, ValueGenerator valueGenerator, object? value)
+    {
+        if (valueGenerator.GeneratesStableValues)
+        {
+            entry[property] = value;
+        }
+        else
+        {
+            entry.SetTemporaryValue(property, value);
+            entry.MarkUnknown(property);
+        }
+    }
+
     private static InternalEntityEntry? TryPropagateValue(InternalEntityEntry entry, IProperty property, IProperty? generationProperty)
     {
         var entityType = entry.EntityType;

diff --git a/src/EFCore/ChangeTracking/Internal/ValueGenerationManager.cs b/src/EFCore/ChangeTracking/Internal/ValueGenerationManager.cs
@@ -57,6 +57,29 @@ public class ValueGenerationManager : IValueGenerationManager
         return chosenPrincipal;
     }
 
+    /// <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
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    public virtual async Task<InternalEntityEntry?> PropagateAsync(InternalEntityEntry entry, CancellationToken cancellationToken)
+    {
+        InternalEntityEntry? chosenPrincipal = null;
+        foreach (var property in entry.EntityType.GetForeignKeyProperties())
+        {
+            if (!entry.HasDefaultValue(property))
+            {
+                continue;
+            }
+
+            var principalEntry = await _keyPropagator.PropagateValueAsync(entry, property, cancellationToken).ConfigureAwait(false);
+            chosenPrincipal ??= principalEntry;
+        }
+
+        return chosenPrincipal;
+    }
+
     /// <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

diff --git a/src/EFCore/Metadata/Internal/PropertyExtensions.cs b/src/EFCore/Metadata/Internal/PropertyExtensions.cs
@@ -136,13 +136,13 @@ public static bool IsKey(this Property property)
     /// </summary>
     public static bool MayBeStoreGenerated(this IProperty property)
     {
-        if (property.ValueGenerated != ValueGenerated.Never)
+        if (property.ValueGenerated != ValueGenerated.Never
+            || property.IsForeignKey())
         {
             return true;
         }
 
-        if (property.IsKey()
-            || property.IsForeignKey())
+        if (property.IsKey())
         {
             var generationProperty = property.FindGenerationProperty();
             return (generationProperty != null)

diff --git a/test/EFCore.Cosmos.FunctionalTests/KeysWithConvertersCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/KeysWithConvertersCosmosTest.cs
@@ -355,8 +355,8 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
                             p => p.Value,
                             p => new Key(p),
                             new ValueComparer<Key>(
-                                (l, r) => l.Value == r.Value,
-                                v => v.Value.GetHashCode()));
+                                (l, r) => (l == null && r == null) || (l != null && r != null && l.Value == r.Value),
+                                v => v == null ? 0 : v.Value.GetHashCode()));
 
                     entity.OwnsOne(p => p.Text);
                     entity.Navigation(p => p.Text).IsRequired();

diff --git a/test/EFCore.InMemory.FunctionalTests/GraphUpdates/GraphUpdatesInMemoryTest.cs b/test/EFCore.InMemory.FunctionalTests/GraphUpdates/GraphUpdatesInMemoryTest.cs
@@ -10,6 +10,10 @@ public GraphUpdatesInMemoryTest(InMemoryFixture fixture)
     {
     }
 
+    // In-memory database does not have database default values
+    public override Task Can_insert_when_composite_FK_has_default_value_for_one_part(bool async)
+        => Task.CompletedTask;
+
     // In-memory database does not have database default values
     public override Task Can_insert_when_FK_has_default_value(bool async)
         => Task.CompletedTask;

diff --git a/test/EFCore.Specification.Tests/CustomConvertersTestBase.cs b/test/EFCore.Specification.Tests/CustomConvertersTestBase.cs
@@ -845,11 +845,15 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
                 {
                     b.HasMany(e => e.Dependents).WithOne(e => e.Principal).HasForeignKey(e => e.PrincipalId);
                     b.Property(e => e.Id).ValueGeneratedNever();
-                    b.Property(e => e.Id).HasConversion(v => v, v => (int)v);
+                    b.Property(e => e.Id).HasConversion<int>(v => v ?? 0, v => v);
                 });
 
             modelBuilder.Entity<NonNullableDependent>(
-                b => b.Property(e => e.Id).ValueGeneratedNever());
+                b =>
+                {
+                    b.Property(e => e.Id).ValueGeneratedNever();
+                    b.Property(e => e.PrincipalId).HasConversion<int>(v => v, v => v);
+                });
 
             modelBuilder.Entity<User>(
                 b =>

diff --git a/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs b/test/EFCore.Specification.Tests/DataAnnotationTestBase.cs
@@ -554,6 +554,19 @@ protected class Profile1
         public virtual Login1 User { get; set; }
     }
 
+    protected class PrincipalA
+    {
+        public int Id { get; set; }
+        public DependantA Dependant { get; set; }
+    }
+
+    protected class DependantA
+    {
+        public int Id { get; set; }
+        public int PrincipalId { get; set; }
+        public PrincipalA Principal { get; set; }
+    }
+
     [ConditionalFact]
     public virtual IModel Key_and_column_work_together()
     {