Stop setting JSON array entities' state to Modified in SaveChanges (#…

…28926) Fixes #28813
dotnet · Sep 1, 2022 · cebba29 · cebba29
1 parent 929cab1
commit cebba29
Show file tree

Hide file tree

Showing 7 changed files with 18 additions and 54 deletions.
diff --git a/src/EFCore.Relational/Update/ModificationCommand.cs b/src/EFCore.Relational/Update/ModificationCommand.cs
@@ -259,25 +259,7 @@ private sealed class JsonPartialUpdateInfo
         public object? PropertyValue { get; set; }
     }
 
-    private record struct JsonPartialUpdatePathEntry
-    {
-        public JsonPartialUpdatePathEntry(
-            string propertyName,
-            int? ordinal,
-            IUpdateEntry parentEntry,
-            INavigation navigation)
-        {
-            PropertyName = propertyName;
-            Ordinal = ordinal;
-            ParentEntry = parentEntry;
-            Navigation = navigation;
-        }
-
-        public string PropertyName { get; }
-        public int? Ordinal { get; }
-        public IUpdateEntry ParentEntry { get; }
-        public INavigation Navigation { get; }
-    }
+    private record struct JsonPartialUpdatePathEntry(string PropertyName, int? Ordinal, IUpdateEntry ParentEntry, INavigation Navigation);
 
     private List<IColumnModification> GenerateColumnModifications()
     {
@@ -334,7 +316,6 @@ private List<IColumnModification> GenerateColumnModifications()
             var processedEntries = new List<IUpdateEntry>();
             foreach (var entry in _entries.Where(e => e.EntityType.IsMappedToJson()))
             {
-                var modifiedMembers = entry.ToEntityEntry().Properties.Where(m => m.IsModified).ToList();
                 var jsonColumn = entry.EntityType.GetContainerColumnName()!;
                 var jsonPartialUpdateInfo = FindJsonPartialUpdateInfo(entry, processedEntries);
 
@@ -423,7 +404,6 @@ private List<IColumnModification> GenerateColumnModifications()
             }
         }
 
-        var processedJsonNavigations = new List<INavigation>();
         foreach (var entry in _entries.Where(x => !x.EntityType.IsMappedToJson()))
         {
             var nonMainEntry = !_mainEntryAdded || entry != _entries[0];
@@ -757,7 +737,7 @@ static JsonPartialUpdateInfo FindCommonJsonPartialUpdateInfo(
             {
                 if (property.IsOrdinalKeyProperty() && ordinal != null)
                 {
-                    entry.SetStoreGeneratedValue(property, ordinal.Value);
+                    entry.SetStoreGeneratedValue(property, ordinal.Value, setModified: false);
                 }
 
                 continue;

diff --git a/src/EFCore/ChangeTracking/Internal/EntityReferenceMap.cs b/src/EFCore/ChangeTracking/Internal/EntityReferenceMap.cs
@@ -228,29 +228,11 @@ public virtual IEnumerable<InternalEntityEntry> GetEntriesForState(
     {
         // Perf sensitive
 
-        var returnAdded
-            = added
-            && _addedReferenceMap != null
-            && _addedReferenceMap.Count > 0;
-
-        var returnModified
-            = modified
-            && _modifiedReferenceMap != null
-            && _modifiedReferenceMap.Count > 0;
-
-        var returnDeleted
-            = deleted
-            && _deletedReferenceMap != null
-            && _deletedReferenceMap.Count > 0;
-
-        var returnUnchanged
-            = unchanged
-            && _unchangedReferenceMap != null
-            && _unchangedReferenceMap.Count > 0;
-
-        var hasSharedTypes
-            = _sharedTypeReferenceMap != null
-            && _sharedTypeReferenceMap.Count > 0;
+        var returnAdded = added && _addedReferenceMap is { Count: > 0 };
+        var returnModified = modified && _modifiedReferenceMap is { Count: > 0 };
+        var returnDeleted = deleted && _deletedReferenceMap is { Count: > 0 };
+        var returnUnchanged = unchanged && _unchangedReferenceMap is { Count: > 0 };
+        var hasSharedTypes = _sharedTypeReferenceMap is { Count: > 0 };
 
         if (!hasSharedTypes)
         {

diff --git a/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs b/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs
@@ -523,8 +523,7 @@ public void SetPropertyModified(
 
         var currentState = _stateData.EntityState;
 
-        if (currentState == EntityState.Added
-            || currentState == EntityState.Detached
+        if (currentState is EntityState.Added or EntityState.Detached
             || !changeState)
         {
             var index = property.GetOriginalValueIndex();
@@ -573,8 +572,7 @@ public void SetPropertyModified(
         }
 
         if (isModified
-            && (currentState == EntityState.Unchanged
-                || currentState == EntityState.Detached))
+            && currentState is EntityState.Unchanged or EntityState.Detached)
         {
             if (changeState)
             {
@@ -746,7 +744,7 @@ public void MarkAsTemporary(IProperty property, bool temporary)
     ///     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 void SetStoreGeneratedValue(IProperty property, object? value)
+    public void SetStoreGeneratedValue(IProperty property, object? value, bool setModified = true)
     {
         if (property.GetStoreGeneratedIndex() == -1)
         {
@@ -758,7 +756,7 @@ public void SetStoreGeneratedValue(IProperty property, object? value)
             property,
             value,
             isMaterialization: false,
-            setModified: true,
+            setModified,
             isCascadeDelete: false,
             CurrentValueType.StoreGenerated);
     }

diff --git a/src/EFCore/Update/IUpdateEntry.cs b/src/EFCore/Update/IUpdateEntry.cs
@@ -108,7 +108,8 @@ public interface IUpdateEntry
     /// </summary>
     /// <param name="property">The property to set the value for.</param>
     /// <param name="value">The value to set.</param>
-    void SetStoreGeneratedValue(IProperty property, object? value);
+    /// <param name="setModified">Whether to set the store-generated property's state to Modified.</param>
+    void SetStoreGeneratedValue(IProperty property, object? value, bool setModified = true);
 
     /// <summary>
     ///     Gets an <see cref="EntityEntry" /> for the entity being saved. <see cref="EntityEntry" /> is an API optimized for

diff --git a/test/EFCore.Cosmos.FunctionalTests/TestUtilities/CosmosTestStore.cs b/test/EFCore.Cosmos.FunctionalTests/TestUtilities/CosmosTestStore.cs
@@ -362,7 +362,7 @@ public void SetOriginalValue(IProperty property, object value)
         public void SetPropertyModified(IProperty property)
             => throw new NotImplementedException();
 
-        public void SetStoreGeneratedValue(IProperty property, object value)
+        public void SetStoreGeneratedValue(IProperty property, object value, bool setModified = true)
             => throw new NotImplementedException();
 
         public EntityEntry ToEntityEntry()

diff --git a/test/EFCore.Relational.Specification.Tests/Update/JsonUpdateTestBase.cs b/test/EFCore.Relational.Specification.Tests/Update/JsonUpdateTestBase.cs
@@ -270,6 +270,9 @@ public virtual Task Add_element_to_json_collection_leaf()
                 entity.OwnedReferenceRoot.OwnedReferenceBranch.OwnedCollectionLeaf.Add(newLeaf);
                 ClearLog();
                 await context.SaveChangesAsync();
+
+                // Do SaveChanges again, see #28813
+                await context.SaveChangesAsync();
             },
             async context =>
             {

diff --git a/test/EFCore.Tests/ExceptionTest.cs b/test/EFCore.Tests/ExceptionTest.cs
@@ -199,7 +199,7 @@ public TProperty GetCurrentValue<TProperty>(IPropertyBase propertyBase)
         public TProperty GetOriginalValue<TProperty>(IProperty property)
             => throw new NotImplementedException();
 
-        public void SetStoreGeneratedValue(IProperty property, object value)
+        public void SetStoreGeneratedValue(IProperty property, object value, bool setModified = true)
             => throw new NotImplementedException();
 
         public EntityEntry ToEntityEntry()