From a36f2e5970d3786bbadb3c855bf962fe82f7c457 Mon Sep 17 00:00:00 2001 From: AndriySvyryd Date: Thu, 4 Jun 2020 16:34:01 -0700 Subject: [PATCH] Add ValueGenerated.OnUpdateSometimes for shared columns Fixes #21139 --- .../Design/CSharpSnapshotGenerator.cs | 4 +- .../Conventions/SharedTableConvention.cs | 19 ++++- .../Internal/InternalEntityEntry.cs | 14 ++-- .../Metadata/Builders/PropertyBuilder.cs | 11 +++ .../Metadata/Internal/PropertyExtensions.cs | 3 +- src/EFCore/Metadata/ValueGenerated.cs | 6 ++ .../Migrations/ModelSnapshotSqlServerTest.cs | 79 +++++++++++++++++++ .../TableSplittingTestBase.cs | 11 +++ 8 files changed, 135 insertions(+), 12 deletions(-) diff --git a/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs b/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs index 12277f70fd0..cff7bab72ab 100644 --- a/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs +++ b/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs @@ -525,7 +525,9 @@ protected virtual void GenerateProperty( ? ".ValueGeneratedOnAdd()" : property.ValueGenerated == ValueGenerated.OnUpdate ? ".ValueGeneratedOnUpdate()" - : ".ValueGeneratedOnAddOrUpdate()"); + : property.ValueGenerated == ValueGenerated.OnUpdateSometimes + ? ".ValueGeneratedOnUpdateSometimes()" + : ".ValueGeneratedOnAddOrUpdate()"); } GeneratePropertyAnnotations(property, stringBuilder); diff --git a/src/EFCore.Relational/Metadata/Conventions/SharedTableConvention.cs b/src/EFCore.Relational/Metadata/Conventions/SharedTableConvention.cs index 278e6fcbf34..52af47d29b6 100644 --- a/src/EFCore.Relational/Metadata/Conventions/SharedTableConvention.cs +++ b/src/EFCore.Relational/Metadata/Conventions/SharedTableConvention.cs @@ -178,8 +178,19 @@ private static void TryUniquifyColumnNames( if ((identifyingMemberInfo != null && identifyingMemberInfo.IsSameAs(otherProperty.PropertyInfo ?? (MemberInfo)otherProperty.FieldInfo)) || (property.IsPrimaryKey() && otherProperty.IsPrimaryKey()) - || (property.IsConcurrencyToken && otherProperty.IsConcurrencyToken)) + || (property.IsConcurrencyToken && otherProperty.IsConcurrencyToken) + || (!property.Builder.CanSetColumnName(null) && !otherProperty.Builder.CanSetColumnName(null))) { + if (property.GetAfterSaveBehavior() == PropertySaveBehavior.Save + && otherProperty.GetAfterSaveBehavior() == PropertySaveBehavior.Save + && property.ValueGenerated == ValueGenerated.Never + && otherProperty.ValueGenerated == ValueGenerated.Never) + { + // Handle this with a default value convention #9329 + property.Builder.ValueGenerated(ValueGenerated.OnUpdateSometimes); + otherProperty.Builder.ValueGenerated(ValueGenerated.OnUpdateSometimes); + } + continue; } @@ -231,7 +242,11 @@ private static string TryUniquify( } columnName = Uniquifier.Uniquify(columnName, properties, maxLength); - property.Builder.HasColumnName(columnName); + if (property.Builder.HasColumnName(columnName) == null) + { + return null; + } + properties[columnName] = property; return columnName; } diff --git a/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs b/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs index 41db4157cb7..441ed9ac267 100644 --- a/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs +++ b/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs @@ -1056,14 +1056,14 @@ public object this[[NotNull] IPropertyBase propertyBase] // Intentionally non-vi var equals = ValuesEqualFunc(property); - if (equals(value, defaultValue)) + if (_storeGeneratedValues.TryGetValue(storeGeneratedIndex, out var generatedValue) + && !equals(generatedValue, defaultValue)) { - if (_storeGeneratedValues.TryGetValue(storeGeneratedIndex, out var generatedValue) - && !equals(generatedValue, defaultValue)) - { - return generatedValue; - } + return generatedValue; + } + if (equals(value, defaultValue)) + { if (_temporaryValues.TryGetValue(storeGeneratedIndex, out generatedValue) && !equals(generatedValue, defaultValue)) { @@ -1100,7 +1100,7 @@ private void SetProperty( bool isCascadeDelete, CurrentValueType valueType) { - var currentValue = this[propertyBase]; + var currentValue = ReadPropertyValue(propertyBase); var asProperty = propertyBase as Property; int propertyIndex; diff --git a/src/EFCore/Metadata/Builders/PropertyBuilder.cs b/src/EFCore/Metadata/Builders/PropertyBuilder.cs index d964b62e24b..2bd3c1262f8 100644 --- a/src/EFCore/Metadata/Builders/PropertyBuilder.cs +++ b/src/EFCore/Metadata/Builders/PropertyBuilder.cs @@ -315,6 +315,17 @@ public virtual PropertyBuilder ValueGeneratedOnUpdate() return this; } + /// + /// Configures a property to have a value generated under certain conditions when saving an existing entity. + /// + /// The same builder instance so that multiple configuration calls can be chained. + public virtual PropertyBuilder ValueGeneratedOnUpdateSometimes() + { + Builder.ValueGenerated(ValueGenerated.OnUpdateSometimes, ConfigurationSource.Explicit); + + return this; + } + /// /// /// Sets the backing field to use for this property. diff --git a/src/EFCore/Metadata/Internal/PropertyExtensions.cs b/src/EFCore/Metadata/Internal/PropertyExtensions.cs index e242c49d9b5..0fad20513cf 100644 --- a/src/EFCore/Metadata/Internal/PropertyExtensions.cs +++ b/src/EFCore/Metadata/Internal/PropertyExtensions.cs @@ -112,8 +112,7 @@ public static bool RequiresValueGenerator([NotNull] this IProperty property) /// public static bool MayBeStoreGenerated([NotNull] this IProperty property) { - if (property.ValueGenerated != ValueGenerated.Never - || property.IsConcurrencyToken) + if (property.ValueGenerated != ValueGenerated.Never) { return true; } diff --git a/src/EFCore/Metadata/ValueGenerated.cs b/src/EFCore/Metadata/ValueGenerated.cs index 428154bf950..727047efb73 100644 --- a/src/EFCore/Metadata/ValueGenerated.cs +++ b/src/EFCore/Metadata/ValueGenerated.cs @@ -36,6 +36,12 @@ public enum ValueGenerated /// OnUpdate = 2, + /// + /// No value is generated when the entity is first added to the database, but a value will be read + /// from the database under certain conditions when the entity is subsequently updated. + /// + OnUpdateSometimes = 4, + /// /// A value is read from the database when the entity is first added and whenever the entity /// is subsequently updated. This is typically used for computed columns and scenarios such as diff --git a/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs b/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs index 4dff8a5fa86..1ab1f8c4450 100644 --- a/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs +++ b/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs @@ -1457,6 +1457,85 @@ public virtual void TableName_preserved_when_generic() }); } + [ConditionalFact] + public virtual void Shared_columns_are_stored_in_the_snapshot() + { + Test( + builder => + { + builder.Entity(b => + { + b.ToTable("EntityWithProperties"); + b.Property("AlternateId").HasColumnName("AlternateId"); + }); + builder.Entity(b => + { + b.ToTable("EntityWithProperties"); + b.Property(e => e.AlternateId).HasColumnName("AlternateId"); + b.HasOne(e => e.EntityWithOneProperty).WithOne(e => e.EntityWithTwoProperties) + .HasForeignKey(e => e.Id); + }); + }, + AddBoilerPlate( + GetHeading() + + @" + modelBuilder.Entity(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+EntityWithOneProperty"", b => + { + b.Property(""Id"") + .ValueGeneratedOnAdd() + .HasColumnType(""int"") + .UseIdentityColumn(); + + b.Property(""AlternateId"") + .ValueGeneratedOnUpdateSometimes() + .HasColumnType(""int"") + .HasColumnName(""AlternateId""); + + b.HasKey(""Id""); + + b.ToTable(""EntityWithProperties""); + }); + + modelBuilder.Entity(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+EntityWithTwoProperties"", b => + { + b.Property(""Id"") + .ValueGeneratedOnAdd() + .HasColumnType(""int"") + .UseIdentityColumn(); + + b.Property(""AlternateId"") + .ValueGeneratedOnUpdateSometimes() + .HasColumnType(""int"") + .HasColumnName(""AlternateId""); + + b.HasKey(""Id""); + + b.ToTable(""EntityWithProperties""); + }); + + modelBuilder.Entity(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+EntityWithTwoProperties"", b => + { + b.HasOne(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+EntityWithOneProperty"", ""EntityWithOneProperty"") + .WithOne(""EntityWithTwoProperties"") + .HasForeignKey(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+EntityWithTwoProperties"", ""Id"") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + + b.Navigation(""EntityWithOneProperty""); + }); + + modelBuilder.Entity(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+EntityWithOneProperty"", b => + { + b.Navigation(""EntityWithTwoProperties""); + });", usingSystem: false), + model => + { + var entityType = model.FindEntityType(typeof(EntityWithOneProperty)); + + Assert.Equal(ValueGenerated.OnUpdateSometimes, entityType.FindProperty("AlternateId").ValueGenerated); + }); + } + [ConditionalFact] public virtual void PrimaryKey_name_preserved_when_generic() { diff --git a/test/EFCore.Relational.Specification.Tests/TableSplittingTestBase.cs b/test/EFCore.Relational.Specification.Tests/TableSplittingTestBase.cs index fc23608057f..b489595e856 100644 --- a/test/EFCore.Relational.Specification.Tests/TableSplittingTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/TableSplittingTestBase.cs @@ -268,6 +268,17 @@ public virtual void Can_use_optional_dependents_with_shared_concurrency_tokens() var scooter = context.Set().Include(v => v.Engine).Single(v => v.Name == "Electric scooter"); Assert.Equal(scooter.SeatingCapacity, context.Entry(scooter.Engine).Property("SeatingCapacity").CurrentValue); + + scooter.SeatingCapacity = 2; + context.SaveChanges(); + } + + using (var context = CreateContext()) + { + var scooter = context.Set().Include(v => v.Engine).Single(v => v.Name == "Electric scooter"); + + Assert.Equal(2, scooter.SeatingCapacity); + Assert.Equal(2, context.Entry(scooter.Engine).Property("SeatingCapacity").CurrentValue); } } }