Add ValueGenerated.OnUpdateSometimes for shared columns

Fixes #21139
dotnet · Aug 26, 2020 · a8f16a9 · a8f16a9
1 parent dc0cdcc
commit a8f16a9
Show file tree

Hide file tree

Showing 12 changed files with 147 additions and 19 deletions.
diff --git 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
@@ -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.Relational/Migrations/Internal/MigrationsModelDiffer.cs b/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs
@@ -1948,7 +1948,8 @@ protected virtual Dictionary<IEntityType, List<ITable>> DiffData(
                         foreach (var targetProperty in entry.EntityType.GetProperties())
                         {
                             if (targetProperty.ValueGenerated != ValueGenerated.Never
-                                && targetProperty.ValueGenerated != ValueGenerated.OnAdd)
+                                && targetProperty.ValueGenerated != ValueGenerated.OnAdd
+                                && targetProperty.ValueGenerated != ValueGenerated.OnUpdateSometimes)
                             {
                                 continue;
                             }

diff --git a/src/EFCore.Relational/Storage/RelationalDatabase.cs b/src/EFCore.Relational/Storage/RelationalDatabase.cs
@@ -53,8 +53,7 @@ public RelationalDatabase(
         /// </summary>
         /// <param name="entries"> Entries representing the changes to be persisted. </param>
         /// <returns> The number of state entries persisted to the database. </returns>
-        public override int SaveChanges(
-            IList<IUpdateEntry> entries)
+        public override int SaveChanges(IList<IUpdateEntry> entries)
         {
             Check.NotNull(entries, nameof(entries));
 

diff --git 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/ChangeTracking/Internal/StateManager.cs b/src/EFCore/ChangeTracking/Internal/StateManager.cs
@@ -304,8 +304,7 @@ public virtual InternalEntityEntry CreateEntry(IDictionary<string, object> value
 
             var valueBuffer = new ValueBuffer(valuesArray);
             var entity = entityType.HasClrType()
-                ? EntityMaterializerSource.GetMaterializer(entityType)(
-                    new MaterializationContext(valueBuffer, Context))
+                ? EntityMaterializerSource.GetMaterializer(entityType)(new MaterializationContext(valueBuffer, Context))
                 : null;
 
             var shadowPropertyValueBuffer = new ValueBuffer(shadowPropertyValuesArray);
@@ -1086,8 +1085,7 @@ private static bool KeyValuesEqual(IProperty property, object value, object curr
         ///     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>
-        protected virtual int SaveChanges(
-            [NotNull] IList<IUpdateEntry> entriesToSave)
+        protected virtual int SaveChanges([NotNull] IList<IUpdateEntry> entriesToSave)
         {
             using (_concurrencyDetector.EnterCriticalSection())
             {

diff --git a/src/EFCore/Metadata/Builders/PropertyBuilder.cs b/src/EFCore/Metadata/Builders/PropertyBuilder.cs
@@ -315,6 +315,17 @@ public virtual PropertyBuilder ValueGeneratedOnUpdate()
             return this;
         }
 
+        /// <summary>
+        ///     Configures a property to have a value generated under certain conditions when saving an existing entity.
+        /// </summary>
+        /// <returns> The same builder instance so that multiple configuration calls can be chained. </returns>
+        public virtual PropertyBuilder ValueGeneratedOnUpdateSometimes()
+        {
+            Builder.ValueGenerated(ValueGenerated.OnUpdateSometimes, ConfigurationSource.Explicit);
+
+            return this;
+        }
+
         /// <summary>
         ///     <para>
         ///         Sets the backing field to use for this property.

diff --git a/src/EFCore/Metadata/Builders/PropertyBuilder`.cs b/src/EFCore/Metadata/Builders/PropertyBuilder`.cs
@@ -232,6 +232,13 @@ public PropertyBuilder([NotNull] IMutableProperty property)
         public new virtual PropertyBuilder<TProperty> ValueGeneratedOnUpdate()
             => (PropertyBuilder<TProperty>)base.ValueGeneratedOnUpdate();
 
+        /// <summary>
+        ///     Configures a property to have a value generated under certain conditions when saving an existing entity.
+        /// </summary>
+        /// <returns> The same builder instance so that multiple configuration calls can be chained. </returns>
+        public new virtual PropertyBuilder<TProperty> ValueGeneratedOnUpdateSometimes()
+            => (PropertyBuilder<TProperty>)base.ValueGeneratedOnUpdateSometimes();
+
         /// <summary>
         ///     <para>
         ///         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
@@ -112,8 +112,7 @@ public static bool RequiresValueGenerator([NotNull] this IProperty property)
         /// </summary>
         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
@@ -36,6 +36,12 @@ public enum ValueGenerated
         /// </summary>
         OnUpdate = 2,
 
+        /// <summary>
+        ///     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.
+        /// </summary>
+        OnUpdateSometimes = 4,
+
         /// <summary>
         ///     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
@@ -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<EntityWithOneProperty>(b =>
+                    {
+                        b.ToTable("EntityWithProperties");
+                        b.Property<int>("AlternateId").HasColumnName("AlternateId");
+                    });
+                    builder.Entity<EntityWithTwoProperties>(b =>
+                    {
+                        b.ToTable("EntityWithProperties");
+                        b.Property<int>(e => e.AlternateId).HasColumnName("AlternateId");
+                        b.HasOne(e => e.EntityWithOneProperty).WithOne(e => e.EntityWithTwoProperties)
+                            .HasForeignKey<EntityWithTwoProperties>(e => e.Id);
+                    });
+                },
+                AddBoilerPlate(
+                    GetHeading()
+                    + @"
+            modelBuilder.Entity(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+EntityWithOneProperty"", b =>
+                {
+                    b.Property<int>(""Id"")
+                        .ValueGeneratedOnAdd()
+                        .HasColumnType(""int"")
+                        .UseIdentityColumn();
+
+                    b.Property<int>(""AlternateId"")
+                        .ValueGeneratedOnUpdateSometimes()
+                        .HasColumnType(""int"")
+                        .HasColumnName(""AlternateId"");
+
+                    b.HasKey(""Id"");
+
+                    b.ToTable(""EntityWithProperties"");
+                });
+
+            modelBuilder.Entity(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+EntityWithTwoProperties"", b =>
+                {
+                    b.Property<int>(""Id"")
+                        .ValueGeneratedOnAdd()
+                        .HasColumnType(""int"")
+                        .UseIdentityColumn();
+
+                    b.Property<int>(""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
@@ -268,6 +268,17 @@ public virtual void Can_use_optional_dependents_with_shared_concurrency_tokens()
                     var scooter = context.Set<PoweredVehicle>().Include(v => v.Engine).Single(v => v.Name == "Electric scooter");
 
                     Assert.Equal(scooter.SeatingCapacity, context.Entry(scooter.Engine).Property<int>("SeatingCapacity").CurrentValue);
+
+                    scooter.SeatingCapacity = 2;
+                    context.SaveChanges();
+                }
+
+                using (var context = CreateContext())
+                {
+                    var scooter = context.Set<PoweredVehicle>().Include(v => v.Engine).Single(v => v.Name == "Electric scooter");
+
+                    Assert.Equal(2, scooter.SeatingCapacity);
+                    Assert.Equal(2, context.Entry(scooter.Engine).Property<int>("SeatingCapacity").CurrentValue);
                 }
             }
         }