Collapse and unify property value behaviors

Part of issue #7913

This change obsoletes ReadOnlyBeforeSave, ReadOnlyAferSave, and StoreGeneratedAlways into two unified facets--one for before-save behavior (Added objects) and one for after-save behavior. In each case the property value can be:
- Ignore: value never sent to database. (Equivalent to StoreGeneratedAlways, but can apply to any property and can be set differently for before and after save.)
- Throw: any non-CLR default value set or property marked as modified causes exception. (Equivalent to previous read-only behaviors.)
- UseValue: the value is used if it has been set (to non-CLR default for store-generated) or marked as modified.

Also covers issue #7914 - makes Ignore the default for before and after save when using AddOrUpdate (computed) properties.

src/EFCore.Relational/Metadata/Internal/RelationalEntityTypeBuilderAnnotations.cs

@@ -243,9 +243,7 @@ private DiscriminatorBuilder DiscriminatorBuilder(
             }
 
             propertyBuilder.IsRequired(true, configurationSource);
-            // TODO: #2132
-            //propertyBuilder.ReadOnlyBeforeSave(true, configurationSource);
-            propertyBuilder.ReadOnlyAfterSave(true, configurationSource);
+            propertyBuilder.AfterSave(PropertyValueBehavior.Throw, configurationSource);
             propertyBuilder.HasValueGenerator(
                 (property, entityType) => new DiscriminatorValueGenerator(GetAnnotations(entityType).DiscriminatorValue),
                 configurationSource);

src/EFCore.Relational/Update/ModificationCommand.cs

@@ -6,6 +6,7 @@
 using System.Linq;
 using JetBrains.Annotations;
 using Microsoft.EntityFrameworkCore.Internal;
+using Microsoft.EntityFrameworkCore.Metadata;
 using Microsoft.EntityFrameworkCore.Metadata.Internal;
 using Microsoft.EntityFrameworkCore.Storage;
 using Microsoft.EntityFrameworkCore.Update.Internal;
@@ -137,7 +138,13 @@ private IReadOnlyList<ColumnModification> GenerateColumnModifications()
                     var isConcurrencyToken = property.IsConcurrencyToken;
                     var isCondition = !adding && (isKey || isConcurrencyToken);
                     var readValue = entry.IsStoreGenerated(property);
-                    var writeValue = !readValue && (adding || entry.IsModified(property));
+
+                    var writeValue = !readValue
+                                     && ((adding
+                                          && property.BeforeSaveBehavior == PropertyValueBehavior.UseValue)
+                                         || (!adding
+                                             && property.AfterSaveBehavior == PropertyValueBehavior.UseValue
+                                             && entry.IsModified(property)));
 
                     if (readValue
                         || writeValue

src/EFCore.Specification.Tests/TestUtilities/Extensions.cs

@@ -81,8 +81,8 @@ private static void CloneProperties(IEntityType sourceEntityType, EntityType tar
                 clonedProperty.IsNullable = property.IsNullable;
                 clonedProperty.IsConcurrencyToken = property.IsConcurrencyToken;
                 clonedProperty.ValueGenerated = property.ValueGenerated;
-                clonedProperty.IsReadOnlyBeforeSave = property.IsReadOnlyBeforeSave;
-                clonedProperty.IsReadOnlyAfterSave = property.IsReadOnlyAfterSave;
+                clonedProperty.BeforeSaveBehavior = property.BeforeSaveBehavior;
+                clonedProperty.AfterSaveBehavior = property.AfterSaveBehavior;
                 property.GetAnnotations().ForEach(annotation => clonedProperty[annotation.Name] = annotation.Value);
             }
         }

src/EFCore.Specification.Tests/TestUtilities/PropertyComparer.cs

@@ -37,8 +37,8 @@ public bool Equals(IProperty x, IProperty y)
                    && x.IsNullable == y.IsNullable
                    && x.IsConcurrencyToken == y.IsConcurrencyToken
                    && x.ValueGenerated == y.ValueGenerated
-                   && x.IsReadOnlyBeforeSave == y.IsReadOnlyBeforeSave
-                   && x.IsReadOnlyAfterSave == y.IsReadOnlyAfterSave
+                   && x.BeforeSaveBehavior == y.BeforeSaveBehavior
+                   && x.AfterSaveBehavior == y.AfterSaveBehavior
                    && (!_compareAnnotations || x.GetAnnotations().SequenceEqual(y.GetAnnotations(), AnnotationComparer.Instance));
         }
 

src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs

@@ -155,7 +155,7 @@ private void SetEntityState(EntityState oldState, EntityState newState, bool acc
                 // Hot path; do not use LINQ
                 foreach (var property in EntityType.GetProperties())
                 {
-                    if (property.IsReadOnlyAfterSave)
+                    if (property.AfterSaveBehavior != PropertyValueBehavior.UseValue)
                     {
                         _stateData.FlagProperty(property.GetIndex(), PropertyFlag.TemporaryOrModified, isFlagged: false);
                     }
@@ -789,7 +789,7 @@ public virtual InternalEntityEntry PrepareToSave()
             {
                 foreach (var property in EntityType.GetProperties())
                 {
-                    if (property.IsReadOnlyBeforeSave
+                    if (property.BeforeSaveBehavior == PropertyValueBehavior.Throw
                         && !HasTemporaryValue(property)
                         && !HasDefaultValue(property))
                     {
@@ -801,7 +801,7 @@ public virtual InternalEntityEntry PrepareToSave()
             {
                 foreach (var property in EntityType.GetProperties())
                 {
-                    if (property.IsReadOnlyAfterSave
+                    if (property.AfterSaveBehavior == PropertyValueBehavior.Throw
                         && IsModified(property))
                     {
                         throw new InvalidOperationException(CoreStrings.PropertyReadOnlyAfterSave(property.Name, EntityType.DisplayName()));
@@ -957,10 +957,13 @@ public virtual void DiscardStoreGeneratedValues()
         public virtual bool IsStoreGenerated(IProperty property)
             => property.ValueGenerated != ValueGenerated.Never
                && ((EntityState == EntityState.Added
-                    && (property.IsStoreGeneratedAlways
+                    && (property.BeforeSaveBehavior == PropertyValueBehavior.Ignore
                         || HasTemporaryValue(property)
                         || HasDefaultValue(property)))
-                   || (property.ValueGenerated == ValueGenerated.OnAddOrUpdate && EntityState == EntityState.Modified && (property.IsStoreGeneratedAlways || !IsModified(property))));
+                   || (property.ValueGenerated == ValueGenerated.OnAddOrUpdate
+                       && EntityState == EntityState.Modified
+                       && (property.AfterSaveBehavior == PropertyValueBehavior.Ignore
+                           || !IsModified(property))));
 
         private bool HasDefaultValue(IProperty property)
             => property.ClrType.IsDefaultValue(this[property]);

src/EFCore/Metadata/IMutableProperty.cs

@@ -1,6 +1,8 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using System;
+
 namespace Microsoft.EntityFrameworkCore.Metadata
 {
     /// <summary>
@@ -28,24 +30,62 @@ public interface IMutableProperty : IProperty, IMutablePropertyBase
         ///     Gets or sets a value indicating when a value for this property will be generated by the database. Even when the
         ///     property is set to be generated by the database, EF may still attempt to save a specific value (rather than
         ///     having one generated by the database) when the entity is added and a value is assigned, or the property is
-        ///     marked as modified for an existing entity. See <see cref="IsStoreGeneratedAlways" /> for more information.
+        ///     marked as modified for an existing entity. See <see cref="BeforeSaveBehavior" /> and <see cref="AfterSaveBehavior"/>
+        ///     for more information.
         /// </summary>
         new ValueGenerated ValueGenerated { get; set; }
 
         /// <summary>
-        ///     Gets or sets a value indicating whether or not this property can be modified before the entity is
-        ///     saved to the database. If true, an exception will be thrown if a value is assigned to
-        ///     this property when it is in the <see cref="EntityState.Added" /> state.
+        ///     <para>
+        ///         Gets a value indicating whether or not this property can be modified before the entity is
+        ///         saved to the database.
+        ///     </para>
+        ///     <para>
+        ///         If <see cref="PropertyValueBehavior.Throw" />, then an exception
+        ///         will be thrown if a value is assigned to this property when it is in
+        ///         the <see cref="EntityState.Added" /> state.
+        ///     </para>
+        ///     <para>
+        ///         If <see cref="PropertyValueBehavior.Ignore" />, then any value
+        ///         set will be ignored when it is in the <see cref="EntityState.Added" /> state.
+        ///     </para>
+        /// </summary>
+        new PropertyValueBehavior BeforeSaveBehavior { get; set; }
+
+        /// <summary>
+        ///     <para>
+        ///         Gets a value indicating whether or not this property can be modified before the entity is
+        ///         saved to the database.
+        ///     </para>
+        ///     <para>
+        ///         If <see cref="PropertyValueBehavior.Throw" />, then an exception
+        ///         will be thrown if a new value is assigned to this property after the entity exists in the database.
+        ///     </para>
+        ///     <para>
+        ///         If <see cref="PropertyValueBehavior.Ignore" />, then any modification to the
+        ///         property value of an entity that already exists in the database will be ignored.
+        ///     </para>
         /// </summary>
+        new PropertyValueBehavior AfterSaveBehavior { get; set; }
+
+        /// <summary>
+        ///     This property is obsolete. Use <see cref="BeforeSaveBehavior"/> instead.
+        /// </summary>
+        [Obsolete("Use BeforeSaveBehavior instead.")]
         new bool IsReadOnlyBeforeSave { get; set; }
 
         /// <summary>
-        ///     Gets or sets a value indicating whether or not this property can be modified after the entity is
-        ///     saved to the database. If true, an exception will be thrown if a new value is assigned to
-        ///     this property after the entity exists in the database.
+        ///     This property is obsolete. Use <see cref="BeforeSaveBehavior"/> instead.
         /// </summary>
+        [Obsolete("Use AfterSaveBehavior instead.")]
         new bool IsReadOnlyAfterSave { get; set; }
 
+        /// <summary>
+        ///     This property is obsolete. Use <see cref="BeforeSaveBehavior"/> or <see cref="AfterSaveBehavior"/> instead.
+        /// </summary>
+        [Obsolete("Use BeforeSaveBehavior or AfterSaveBehavior instead.")]
+        new bool IsStoreGeneratedAlways { get; set; }
+
         /// <summary>
         ///     Gets or sets a value indicating whether this property is used as a concurrency token. When a property is configured
         ///     as a concurrency token the value in the database will be checked when an instance of this entity type
@@ -54,14 +94,5 @@ public interface IMutableProperty : IProperty, IMutablePropertyBase
         ///     changes will not be applied to the database.
         /// </summary>
         new bool IsConcurrencyToken { get; set; }
-
-        /// <summary>
-        ///     Gets or sets a value indicating whether or not the database will always generate a value for this property.
-        ///     If set to true, a value will always be read back from the database whenever the entity is saved
-        ///     regardless of the state of the property. If set to false, whenever a value is assigned to the property
-        ///     (or marked as modified) EF will attempt to save that value to the database rather than letting the
-        ///     database generate one.
-        /// </summary>
-        new bool IsStoreGeneratedAlways { get; set; }
     }
 }

src/EFCore/Metadata/IProperty.cs

@@ -23,33 +23,62 @@ public interface IProperty : IPropertyBase
         bool IsNullable { get; }
 
         /// <summary>
-        ///     Gets a value indicating whether or not this property can be modified before the entity is
-        ///     saved to the database. If true, an exception will be thrown if a value is assigned to
-        ///     this property when it is in the <see cref="EntityState.Added" /> state.
+        ///     <para>
+        ///         Gets a value indicating whether or not this property can be modified before the entity is
+        ///         saved to the database.
+        ///     </para>
+        ///     <para>
+        ///         If <see cref="PropertyValueBehavior.Throw" />, then an exception
+        ///         will be thrown if a value is assigned to this property when it is in
+        ///         the <see cref="EntityState.Added" /> state.
+        ///     </para>
+        ///     <para>
+        ///         If <see cref="PropertyValueBehavior.Ignore" />, then any value
+        ///         set will be ignored when it is in the <see cref="EntityState.Added" /> state.
+        ///     </para>
         /// </summary>
+        PropertyValueBehavior BeforeSaveBehavior { get; }
+
+        /// <summary>
+        ///     <para>
+        ///         Gets a value indicating whether or not this property can be modified after the entity is
+        ///         saved to the database.
+        ///     </para>
+        ///     <para>
+        ///         If <see cref="PropertyValueBehavior.Throw" />, then an exception
+        ///         will be thrown if a new value is assigned to this property after the entity exists in the database.
+        ///     </para>
+        ///     <para>
+        ///         If <see cref="PropertyValueBehavior.Ignore" />, then any modification to the
+        ///         property value of an entity that already exists in the database will be ignored.
+        ///     </para>
+        /// </summary>
+        PropertyValueBehavior AfterSaveBehavior { get; }
+
+        /// <summary>
+        ///     This property is obsolete. Use <see cref="BeforeSaveBehavior"/> instead.
+        /// </summary>
+        [Obsolete("Use BeforeSaveBehavior instead.")]
         bool IsReadOnlyBeforeSave { get; }
 
         /// <summary>
-        ///     Gets a value indicating whether or not this property can be modified after the entity is
-        ///     saved to the database. If true, an exception will be thrown if a new value is assigned to
-        ///     this property after the entity exists in the database.
+        ///     This property is obsolete. Use <see cref="BeforeSaveBehavior"/> instead.
         /// </summary>
+        [Obsolete("Use AfterSaveBehavior instead.")]
         bool IsReadOnlyAfterSave { get; }
 
         /// <summary>
-        ///     Gets a value indicating whether or not the database will always generate a value for this property.
-        ///     If set to true, a value will always be read back from the database whenever the entity is saved
-        ///     regardless of the state of the property. If set to false, whenever a value is assigned to the property
-        ///     (or marked as modified) EF will attempt to save that value to the database rather than letting the
-        ///     database generate one.
+        ///     This property is obsolete. Use <see cref="BeforeSaveBehavior"/> or <see cref="AfterSaveBehavior"/> instead.
         /// </summary>
+        [Obsolete("Use BeforeSaveBehavior or AfterSaveBehavior instead.")]
         bool IsStoreGeneratedAlways { get; }
 
         /// <summary>
         ///     Gets a value indicating when a value for this property will be generated by the database. Even when the
         ///     property is set to be generated by the database, EF may still attempt to save a specific value (rather than
         ///     having one generated by the database) when the entity is added and a value is assigned, or the property is
-        ///     marked as modified for an existing entity. See <see cref="IsStoreGeneratedAlways" /> for more information.
+        ///     marked as modified for an existing entity. See <see cref="BeforeSaveBehavior" /> and <see cref="AfterSaveBehavior"/>
+        ///     for more information.
         /// </summary>
         ValueGenerated ValueGenerated { get; }
 

src/EFCore/Metadata/Internal/EntityTypeExtensions.cs

@@ -461,7 +461,8 @@ public static IEnumerable<IPropertyBase> GetNotificationProperties(
         {
             if (string.IsNullOrEmpty(propertyName))
             {
-                foreach (var property in entityType.GetProperties().Where(p => !p.IsReadOnlyAfterSave))
+                foreach (var property in entityType.GetProperties()
+                    .Where(p => p.AfterSaveBehavior == PropertyValueBehavior.UseValue))
                 {
                     yield return property;
                 }