From 29b62d87d7e9bf38f09d3f75f1ff9a58d9f87f10 Mon Sep 17 00:00:00 2001 From: Andriy Svyryd Date: Fri, 4 Mar 2022 13:12:07 -0800 Subject: [PATCH] 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 --- .../Internal/InternalEntityEntry.cs | 6 +- .../Metadata/Internal/PropertyExtensions.cs | 10 ++- .../StoreGeneratedTestBase.cs | 67 +++++++++++++++++-- .../StoreGeneratedSqlServerTest.cs | 2 + 4 files changed, 79 insertions(+), 6 deletions(-) diff --git a/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs b/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs index 31fd7530011..a5afeb7d103 100644 --- 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; + /// /// 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 index 53826c3b025..139bef01d31 100644 --- 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; + /// /// 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; /// diff --git a/test/EFCore.Specification.Tests/StoreGeneratedTestBase.cs b/test/EFCore.Specification.Tests/StoreGeneratedTestBase.cs index d40ff4536d8..767562ced5a 100644 --- 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().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 Periods { get; } = new HashSet(); + } + + 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().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(); modelBuilder.Entity(); - modelBuilder.Entity() - .Property(e => e.HasTemp) - .ValueGeneratedOnAddOrUpdate() - .HasValueGenerator(); + modelBuilder.Entity(eb => + { + eb.Property(e => e.HasTemp) + .ValueGeneratedOnAddOrUpdate() + .HasValueGenerator(); + }); + + modelBuilder.Entity(entity => + { + entity.HasKey(x => x.Id); + entity.Property(x => x.Id) + .ValueGeneratedOnAdd(); + entity.HasOne(x => x.Current) + .WithOne() + .HasForeignKey(x => new { x.Id, x.CurrentNumber }); + }); + + modelBuilder.Entity(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 index 6726e148e09..0ac152f55b9 100644 --- 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().Property(e => e.HasTemp).HasDefaultValue(777); + modelBuilder.Entity().Property(e => e.Id).UseIdentityColumn(); + base.OnModelCreating(modelBuilder, context); } }